Don't fail on missing system config. #2582
Conversation
| return fmt.Errorf("no system config available") | ||
| // This could be before the system config was ever gossiped, | ||
| // or it expired. Let the gossip callback set the info. | ||
| log.Warningf("no system config available, cannot determine range MaxBytes") |
There was a problem hiding this comment.
can you make this an errors.New and check equality in the caller? That would make me feel better than returning nil.
There was a problem hiding this comment.
why? we're now saying this is no longer an error, merely an acceptable possibility.
There was a problem hiding this comment.
well, because in this case updateRangeInfo didn't actually do its work, so it did not complete without error. I think what we're saying is that failure in this function is sometimes acceptable. We can either model that as a particular error that is safe to ignore, or break the function into two: one which is allowed to fail, and one which is not.
There was a problem hiding this comment.
this is the last call to GetSystemConfig that throws an error. Everything else just logs a message. Just like failing on this is odd, so would returning an error just in this case.
There was a problem hiding this comment.
I'm not sold; I'd rather this be broken into two functions (or just change this function to take cfg as an argument) with differing error semantics.
LGTM as-is for the purpose of fixing the flaky tests.
|
Is there technically a race here in which |
|
splits are fine. Not sure about snapshots, I haven't traced through those, but as long as replicas are added to the store first, they'll be fine as well. |
|
yeah, |
Fixes #2566 When the system config is not loaded, MaxBytes will remain 0 and be ignored for split purposes. It will be properly updated the next time the system config callback runs.
mberhault
force-pushed
the
marc/ignore_missing_config
branch
from
a0e037b to
7febfd9
Compare
|
Really seeing a ton of these now. |
Don't fail on missing system config.
|
Filed #2649 |
Fixes #2566
When the system config is not loaded, MaxBytes will remain 0 and
be ignored for split purposes.
It will be properly updated the next time the system config callback
runs.