Follow-up from PR #820 review (thread on backfill/process.go, comment 3515787395).
The nil-logger positional mode of lifecycle.LastCommittedLedger(cat, logger) is production-dead (the only production caller, startup.go, passes a real logger; the tick's LastCommittedLedger(cat, nil) went away with the #25 clamp deletion). It should be removed, along with the logger parameter, landing the design-pseudocode signature LastCommittedLedger(cat).
Blocked on two coupled changes, which is why it wasn't folded into the review round:
- The
TestLastCommittedLedger unit tests use the nil-logger mode as a seam to exercise the positional derivation WITHOUT opening a real hot DB. They must be converted to real-DB tests (as TestDeriveWatermark_RealHotDB_RefinementIsNotStale already is) or the positional-only cases dropped.
- The refinement's read-only open needs a logger; drop the param by defaulting nil->nop in
rocksdb.Config (the same idiom orNop applies to MetricSink), or retain a logger on Catalog.
Pure cleanup — no behavior change.
Follow-up from PR #820 review (thread on backfill/process.go, comment 3515787395).
The nil-logger positional mode of
lifecycle.LastCommittedLedger(cat, logger)is production-dead (the only production caller, startup.go, passes a real logger; the tick'sLastCommittedLedger(cat, nil)went away with the #25 clamp deletion). It should be removed, along with theloggerparameter, landing the design-pseudocode signatureLastCommittedLedger(cat).Blocked on two coupled changes, which is why it wasn't folded into the review round:
TestLastCommittedLedgerunit tests use the nil-logger mode as a seam to exercise the positional derivation WITHOUT opening a real hot DB. They must be converted to real-DB tests (asTestDeriveWatermark_RealHotDB_RefinementIsNotStalealready is) or the positional-only cases dropped.rocksdb.Config(the same idiomorNopapplies to MetricSink), or retain a logger onCatalog.Pure cleanup — no behavior change.