db: add span policy enforcer background goroutine#5758
db: add span policy enforcer background goroutine#5758xinhaoz wants to merge 2 commits intocockroachdb:masterfrom
Conversation
annrpom
left a comment
There was a problem hiding this comment.
@annrpom reviewed 8 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @xinhaoz).
span_policy_enforcer.go line 96 at r2 (raw file):
} nextFile, level, endOfScan := s.getNextFile(true /* waitForPending */)
nit: waitForPendingWork
595937b to
fa5c524
Compare
Potential Bug(s) DetectedThe three-stage Claude Code analysis has identified potential bug(s) in this PR that may warrant investigation. Next Steps: Note: When viewing the workflow output, scroll to the bottom to find the Final Analysis Summary. After you review the findings, please tag the issue as follows:
|
fa5c524 to
49ad43e
Compare
RaduBerinde
left a comment
There was a problem hiding this comment.
@RaduBerinde made 4 comments.
Reviewable status: 2 of 8 files reviewed, 5 unresolved discussions (waiting on @annrpom and @xinhaoz).
internal/manifest/scan_cursor.go line 87 at r3 (raw file):
// position. This is useful for skipping files that have already been processed. func (c *ScanCursor) FileIsAfterCursor(cmp base.Compare, f *TableMetadata, level int) bool { return c.Compare(cmp, MakeScanCursorAfterFile(f, level)) < 0
[nit] this is equivalent to c.Compare(cmp, MakeScanCursor(f, level)) <= 0, correct? I think that would be more clear.
internal/manifest/scan_cursor.go line 172 at r3 (raw file):
} // nextFileOnLevel returns the first file on c.Level which is at or after the
which is after the cursor position
internal/manifest/scan_cursor.go line 176 at r3 (raw file):
func (c *ScanCursor) nextFileOnLevel(cmp base.Compare, v *Version) *TableMetadata { if c.Level == 0 { return c.nextFileOnL0(v)
This is not doing what we advertise - the next file in L0 in terms of seqnums is not the first file on 0 which is at or after the cursor. We would have to look on each sublevel, similar to NextExternalFileOnLevel
internal/manifest/scan_cursor.go line 184 at r3 (raw file):
f := it.SeekGE(cmp, c.Key) // Find the first file at or after the cursor position. for f != nil && !c.FileIsAfterCursor(cmp, f, c.Level) {
[nit] "Skip the file if it starts before cursor.Key or is at that same key with lower sequence number." explains better
Add NextFile function to ScanCursor, which returns the first file after the cursor position.
Add a background span policy enforcer that scans the LSM to detect files violating compression policies and marks them for policy enforcement compaction (a new compaction type that will be introduced in a later commit). Currently disabled by default, this initial implementation scans at a rate of 1 file per second, and checks only for a violation on the compression settings in the span policy. Part of: cockroachdb#5657
49ad43e to
6e071ac
Compare
xinhaoz
left a comment
There was a problem hiding this comment.
@xinhaoz made 3 comments and resolved 3 discussions.
Reviewable status: 1 of 8 files reviewed, 2 unresolved discussions (waiting on @annrpom).
span_policy_enforcer.go line 96 at r2 (raw file):
Previously, annrpom (annie pompa) wrote…
nit: waitForPendingWork
Done.
internal/manifest/scan_cursor.go line 87 at r3 (raw file):
Previously, RaduBerinde wrote…
[nit] this is equivalent to
c.Compare(cmp, MakeScanCursor(f, level)) <= 0, correct? I think that would be more clear.
Done.
internal/manifest/scan_cursor.go line 176 at r3 (raw file):
Previously, RaduBerinde wrote…
This is not doing what we advertise - the next file in L0 in terms of seqnums is not the first file on 0 which is at or after the cursor. We would have to look on each sublevel, similar to
NextExternalFileOnLevel
Done.
manifest: add NextFile functions to ScanCursor
Add NextFile function to ScanCursor, which returns the first file after the
cursor position.
db: add span policy enforcer background goroutine
Add a background span policy enforcer that scans the LSM to detect files
violating compression policies and marks them for policy enforcement compaction
(a new compaction type that will be introduced in a later commit).
Currently disabled by default, this initial implementation scans at a rate of
1 file per second, and checks only for a violation on the compression settings
in the span policy.
Part of: #5657