sql-server: Add behavior: auto_gc_behavior: enable. #8849
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When Auto GC is enabled, the running sql-server will periodically collect a Dolt database that is growing in size. This behavior is currently experimental. Tuning the behavior around how often to collect is ongoing work.
…s to be more reliable.
… same database as the auto GC work.
|
@coffeegoddd DOLT
|
zachmu
approved these changes
Feb 13, 2025
| // delivers when no GC is currently running. | ||
| done chan struct{} | ||
| // It simplifies the logic and efficiency of the | ||
| // implementation a bit to have a |
| type autoGCCommitHook struct { | ||
| c *AutoGCController | ||
| name string | ||
| // Always non-nil, if this channel delivers this channel |
…ation to also auto-gc.
…test for auto-gc occurring on a standby replica.
…cks, plus gets triggered by a commit hook. This helps handle the stanbdy replica case, where commits come in through remotesrv directly into the ChunkStore, and not through the datas.Database.
… certain errros that arise after a GC.
…s on waitForGC retry.
…mount of garbage produced by replication test to improve speed.
|
@coffeegoddd DOLT
|
zachmu
approved these changes
Feb 24, 2025
| }, nil | ||
| } | ||
| } else { | ||
| totalSz, err := cs.(chunks.TableFileStore).Size(ctx) |
There was a problem hiding this comment.
Worth adding an ok check for this type assertion, rather than panic?
| if h.lastSz == nil { | ||
| h.lastSz = &sz | ||
| } | ||
| const size_128mb = (1 << 27) |
There was a problem hiding this comment.
Threshold values probably should be defined as constants.
Might also consider making these settable via environment vars. A reasonable testing strategy might be running benchmarks / other heavy IO processes multithreaded with a quite low threshold and making sure perf is reasonable / no deadlock happen.
There was a problem hiding this comment.
Anything that needs to be tuneable is gonna be tuneable via config.yaml eventually, but this first pass just has them hard coded...
Same for the timer on the periodic check.
| return nil | ||
| } | ||
|
|
||
| const checkInterval = 100 * time.Millisecond |
There was a problem hiding this comment.
This seems really aggressive. Any reason to be this frequent by default?
Also, make configurable via env?
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
|
@coffeegoddd DOLT
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When Auto GC is enabled, the running sql-server will periodically collect a Dolt database that is growing in size. This behavior is currently experimental. Tuning the behavior around how often to collect is ongoing work.