Fix garbage collector interval and backoff resets #1663
Conversation
github-actions
bot
added
area/datastore
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
| @@ -111,6 +111,7 @@ func startGarbageCollectorWithMaxElapsedTime(ctx context.Context, gc GarbageColl | |||
| backoffInterval.InitialInterval = interval | |||
| backoffInterval.MaxInterval = max(MaxGCInterval, interval) | |||
| backoffInterval.MaxElapsedTime = maxElapsedTime | |||
| backoffInterval.Reset() | |||
There was a problem hiding this comment.
This is the other issue I discovered during testing. Without this, the initial interval used by the backoff will be the default 500ms until backoffInterval.Reset() is called. This is an edge case, but it can cause the wrong initial value to be used if the first iteration of the GC has an error.
| type testGC struct{} | ||
| // Fake garbage collector that returns a new incremented revision each time | ||
| // TxIDBefore is called. | ||
| type fakeGC struct { |
There was a problem hiding this comment.
I updated the testGC to be more fake-like so it could be used to test different scenarios. In my case, I need it to intermittently fail and then recover. If you'd prefer a special, more straightforward GC implementation just for the new test, I'm happy to change it.
There was a problem hiding this comment.
I think this is great 👍🏻
|
I have read the CLA Document and I hereby sign the CLA |
| type testGC struct{} | ||
| // Fake garbage collector that returns a new incremented revision each time | ||
| // TxIDBefore is called. | ||
| type fakeGC struct { |
There was a problem hiding this comment.
I think this is great 👍🏻
Fixes #1659
Resets
nextIntervalback to the configured interval whenbackoffInterval.Reset()is called on a successful garbage collection run.It also fixes a different problem I discovered while writing tests.
backoffInterval.Reset()must be called after initialization according to these docs:When this is not done, the
InitialIntervalvalue is ignored, and the default500msis used until the first GC iteration completes successfully andbackoffInterval.Reset()is called. I assume this is unintentional behaviour?I haven't written much Go, so please feel free to nitpick the PR. I'm open to any suggestions. 😅