-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft: perform index maintenance on repo maintenance #534
Draft: perform index maintenance on repo maintenance #534
Conversation
3dfa75d
to
c6ad13b
Compare
c6386c7
to
d689427
Compare
|
||
if shouldAdvance(cs.UncompactedEpochSets[cs.WriteEpoch], p.MinEpochDuration, p.EpochAdvanceOnCountThreshold, p.EpochAdvanceOnTotalSizeBytesThreshold) { | ||
return errors.Wrap(e.advanceEpochMarker(ctx, cs), "error advancing epoch") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't recall this 100%, did we decide that we should advance epoch during maintenance only or that will continue to happen during index loads on write connection ?
In other words, with this change e.allowWrites()
will skip epoch advance here by-default now since allowWrites()
is going to be returning false
always until someone overrides the index manager config from CLI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I just noticed that MaybeAdvanceWriteEpoch()
is exposed to be called from maintenance only. That makes sense, but what if maintenance is enabled and e.allowWrites()
returns false ? Would that ever happen, if not is that check needed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch, changed.
c6ad13b
to
90a0e51
Compare
d689427
to
941d744
Compare
90a0e51
to
c406533
Compare
67984c6
to
b63dac9
Compare
c406533
to
065cf05
Compare
fdd22fe
to
19f504d
Compare
d4a7004
to
84da027
Compare
19f504d
to
f75914d
Compare
f75914d
to
8f98f4e
Compare
Closing in favor of the upstream PR kopia#3651 |
Early draft with partial implementation