feat(dataobj): Schedule LogMerge via per-tenant IndexMerge/LogMerge flip-flop#23316
Open
trevorwhitney wants to merge 7 commits into
Open
feat(dataobj): Schedule LogMerge via per-tenant IndexMerge/LogMerge flip-flop#23316trevorwhitney wants to merge 7 commits into
trevorwhitney wants to merge 7 commits into
Conversation
This file contains hidden or 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
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.
What this PR does
Changes how dataobj log compaction (
LogMerge) is scheduled. PreviouslyLogMergeonly ran once a(tenant, window)had converged to a single index (len(entries) == 1routing in the old poll loop). Under continuous ingestion a window routinely holds more than one index, so it never converged and its logs were never compacted.This replaces the cron/poll loop with a per-tenant IndexMerge ↔ LogMerge flip-flop:
coordinator.Runspawns one worker goroutine per configured tenant (-dataobj.compaction.tenants) and drains them via aWaitGroupon shutdown. There is no poll loop.IndexMerge(unchangedcompactTenant) thenLogMerge. On success it flips; on error it re-arms the same phase.LogMergeno longer waits for convergence — it compacts the log objects behind every index in the window, one index at a time, reusing the existingcompactTenantLogsper index. This is the core fix.Why
LogMergeeffectively never triggered in environments that don't converge to a single index. Decoupling it from convergence lets log compaction make progress regardless of index count.Scope / notes
Targeted at getting
LogMergerunning in a dev environment for a single configured tenant. Deliberately deferred (documented for revisit before broader/production use):PollingIntervalis retained as a config knob but no longer read by the scheduler (reserved for a future inter-phase throttle).compacted|converged|failed).Config
-dataobj.compaction.tenants(comma-separated) — the tenants to compact; one worker each. Static (restart to change); empty = no workers.Testing
phaseOutcomemapping, cancellation short-circuit), plus lifecycle tests forRun/runTenantLoop(WaitGroup drain on cancel, error-keeps-phase / success-flips, tenant dedupe).go test ./pkg/engine/... ./pkg/dataobj/...green;go vetandgofmtclean.Operational acceptance (dev)
With
-dataobj.compaction.enabled=true -dataobj.compaction.tenants=<tenant>,loki_dataobj_compaction_tenant_log_cycles_total{outcome="compacted"}should increment on windows with >1 index (which previously never happened), and..._indexes_added_total/..._tasks_totaladvance during log-merge phases.