Page MenuHomePhabricator

Use "null", not "-1", as a local "no version" marker when performing intracluster repository sync

Authored by epriestley on Jan 28 2019, 7:31 PM.



Ref T13242. See

The synchronization log column is uint32? and -1 doesn't go into that column.

Since we're only using -1 for convenience to cheat through $max_version > $this_version checks, use null instead and make the checks more explicit.

Test Plan

Reproducing this is a bit tricky and I cheated fairly heavily to force the code down this pathway without actually building a multi-device cluster, but I did reproduce the original exception, apply the patch, and observe that it fixed things.

Diff Detail

rP Phabricator
Lint OK
Unit Tests OK
Build Status
Buildable 21726
Build 29643: Run Core Tests
Build 29642: arc lint + arc unit

Event Timeline

epriestley created this revision.Jan 28 2019, 7:31 PM
epriestley requested review of this revision.Jan 28 2019, 7:33 PM
epriestley added inline comments.Jan 28 2019, 7:34 PM

I also traced this variable down the stack, nothing else uses it except the logging which is running into an issue.

Thanks for fixing this. When I get back let's make sure I can repro these cluster issues in my dev environment and update the docs (and/or finish my workflow script for automatically setting up all this state for dev environments, which is arguably better than documentation).

amckinley accepted this revision.Jan 29 2019, 2:44 AM
This revision is now accepted and ready to land.Jan 29 2019, 2:44 AM

I think reproducing this "legitimately" ultimately requires genuinely setting up two actual repository hosts so it's probably not reproducible in a local Phacility development configuration. I'm not sure what we want to do about that long-term, maybe spinning up a private cluster as a testbed is the best approach to look towards.

(We should definitely get your dev environment to a state where repository cluster stuff works, though, since 95% of stuff doesn't need 2+ hosts to repro/test/develop.)

This revision was automatically updated to reflect the committed changes.