chore(amber): remove the unused EmptyReplayLogger

[aglinxinyuan]

What changes were proposed in this PR?

Deletes EmptyReplayLogger and its spec — a no-op ReplayLogger that nothing in the engine ever constructs. Pure deletion, no behaviour change: −171 lines.

The live null object for log replay sits one layer up, at the manager. EmptyReplayLogManagerImpl inherits the ReplayLogManager trait's non-logging withFaultTolerant, so it never needs a logger at all:

ReplayLogManager.scala:51  -> EmptyReplayLogManagerImpl   (no-op manager, live)
ReplayLogManager.scala:109 -> new ReplayLoggerImpl()      (the only ReplayLogger built in main)
                              EmptyReplayLogger           (never instantiated)

69b7f9bc1 (#2230) added ReplayLogger, ReplayLoggerImpl, EmptyReplayLogger and ReplayLogManager in a single commit, so nothing ever moved layers — EmptyReplayLogger was unwired from the day it landed. No configuration selects a logger class by name either, so there is no path that revives it.

Reviewer note: the abstract base ReplayLogger is not touched — ReplayLoggerImpl extends it and is unaffected. It does end up with a single subclass and no declared-type site once this lands, so collapsing it is tracked separately in #7673; that one edits live code, while this PR stays a pure dead-code deletion. EmptyReplayLogger acquired unit tests during the 2026 coverage work (#5554), which is why it currently looks live; the spec covers this class and nothing else.

Any related issues, documentation, discussions?

Closes #7449

How was this PR tested?

Existing tests only — this PR adds none, since it removes code and the spec that covered it.

Locally, from the repo root with Java 17:

• sbt "WorkflowExecutionService/Test/compile" — success.

Verification, re-runnable by a reviewer:

git grep -n EmptyReplayLogger                          # only the two deleted files
git grep -n "extends ReplayLogger" -- amber/src/main   # ReplayLoggerImpl remains
git log -S "new EmptyReplayLogger" --all               # only the #5554 spec commits

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Claude Opus 5)

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

• No candidates found from git blame history.

[github-actions]

⚠️ Benchmark changes need a look

🟢 0 better · 🔴 6 worse · ⚪ 9 noise (<±5%) · 0 without baseline

Compared against main e03d971 benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

	config	throughput	MB/s	latency	max Δ latest / 7d
🔴	bs=10 sw=10 sl=64	376	0.229	27,068/37,552/37,552 us	🔴 +15.3% / 🔴 +133.0%
🔴	bs=100 sw=10 sl=64	777	0.474	128,531/158,524/158,524 us	🔴 +6.6% / 🔴 +42.5%
⚪	bs=1000 sw=10 sl=64	912	0.556	1,091,646/1,151,546/1,151,546 us	⚪ within ±5% / 🔴 +8.4%

Baseline details

Latest main e03d971 from same runner

config	metric	PR	latest main	7d avg	Δ latest	Δ 7d
bs=10 sw=10 sl=64	throughput	376 tuples/sec	398 tuples/sec	752.25 tuples/sec	-5.5%	-50.0%
bs=10 sw=10 sl=64	MB/s	0.229 MB/s	0.243 MB/s	0.459 MB/s	-5.8%	-50.1%
bs=10 sw=10 sl=64	p50	27,068 us	23,470 us	13,006 us	+15.3%	+108.1%
bs=10 sw=10 sl=64	p95	37,552 us	37,192 us	16,117 us	+1.0%	+133.0%
bs=10 sw=10 sl=64	p99	37,552 us	37,192 us	19,286 us	+1.0%	+94.7%
bs=100 sw=10 sl=64	throughput	777 tuples/sec	826 tuples/sec	953.66 tuples/sec	-5.9%	-18.5%
bs=100 sw=10 sl=64	MB/s	0.474 MB/s	0.504 MB/s	0.582 MB/s	-6.0%	-18.6%
bs=100 sw=10 sl=64	p50	128,531 us	120,577 us	105,130 us	+6.6%	+22.3%
bs=100 sw=10 sl=64	p95	158,524 us	152,161 us	111,226 us	+4.2%	+42.5%
bs=100 sw=10 sl=64	p99	158,524 us	152,161 us	121,065 us	+4.2%	+30.9%
bs=1000 sw=10 sl=64	throughput	912 tuples/sec	917 tuples/sec	984.25 tuples/sec	-0.5%	-7.3%
bs=1000 sw=10 sl=64	MB/s	0.556 MB/s	0.56 MB/s	0.601 MB/s	-0.7%	-7.4%
bs=1000 sw=10 sl=64	p50	1,091,646 us	1,085,417 us	1,024,295 us	+0.6%	+6.6%
bs=1000 sw=10 sl=64	p95	1,151,546 us	1,185,028 us	1,062,710 us	-2.8%	+8.4%
bs=1000 sw=10 sl=64	p99	1,151,546 us	1,185,028 us	1,095,400 us	-2.8%	+5.1%

Raw CSV

config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,532.11,200,128000,376,0.229,27068.39,37551.70,37551.70
1,100,10,64,20,2574.97,2000,1280000,777,0.474,128531.45,158524.01,158524.01
2,1000,10,64,20,21940.33,20000,12800000,912,0.556,1091646.25,1151545.56,1151545.56

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.32%. Comparing base (e03d971) to head (ac3a565).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #7452      +/-   ##
============================================
- Coverage     85.32%   85.32%   -0.01%     
+ Complexity     4151     4149       -2     
============================================
  Files          1169     1168       -1     
  Lines         46740    46738       -2     
  Branches       5202     5202              
============================================
- Hits          39882    39880       -2     
  Misses         5148     5148              
  Partials       1710     1710              

Flag	Coverage Δ		*Carryforward flag
access-control-service	70.00% <ø> (ø)		Carriedforward from e03d971
agent-service	85.50% <ø> (ø)		Carriedforward from e03d971
amber	80.88% <ø> (-0.01%)	⬇️
computing-unit-managing-service	50.72% <ø> (ø)		Carriedforward from e03d971
config-service	65.97% <ø> (ø)		Carriedforward from e03d971
file-service	69.05% <ø> (ø)		Carriedforward from e03d971
frontend	87.85% <ø> (ø)		Carriedforward from e03d971
notebook-migration-service	78.89% <ø> (ø)		Carriedforward from e03d971
pyamber	97.51% <ø> (ø)		Carriedforward from e03d971
workflow-compiling-service	26.31% <ø> (ø)		Carriedforward from e03d971

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Yicong-Huang]

I also think this looks unused. @shengquan-ni can you help confirm?

[Yicong-Huang]

🟡 0 must-fix · 2 advisory · 0 polish — the deletion is safe; both advisories are about what to say and what to do next, not the diff.

Simplifications (1)

• ReplayLogger.scala:28 — the base now has one subclass and no declared-type site left in the repo; a follow-up removal candidate under the "abstractions after the second use" standard (advisory).

Conventions (1)

• The description says the null-object role "moved up a layer"; 69b7f9bc1 added both classes in one commit, so nothing moved — it was never wired (advisory). The corrected story is stronger for the PR.

Verification trace

Confirming the open thread's question — whether this is really unused: git log -S "new EmptyReplayLogger" --all returns only the 2026 spec commits and this removal, so no production commit ever constructed it. The live null object is selected one layer up (ReplayLogManager.scala:51), and EmptyReplayLogManagerImpl inherits the trait's non-logging withFaultTolerant, so it never needed a logger at all. git grep EmptyReplayLogger is empty at the head and on current main, and nothing persists an instance — ReplayLogger is not Serializable and its manager is @transient.

[aglinxinyuan]

Confirmed unused, with history to back it: git log -S "new EmptyReplayLogger" --all returns only the #5554 spec commits and this removal, so no production commit ever constructed it. The live null object is EmptyReplayLogManagerImpl one layer up, which inherits the trait's non-logging withFaultTolerant and needs no logger.

Both advisories addressed:

• Corrected the description — it claimed the null-object role "moved up a layer", but 69b7f9bc1 (Refactor logging for fault tolerance #2230) added all four classes in one commit, so EmptyReplayLogger was simply never wired.
• ReplayLogger is indeed down to one subclass with no declared-type site. Filed as Collapse the single-subclass ReplayLogger base into ReplayLoggerImpl #7673 rather than folded in here, since collapsing it edits live code while this PR is a pure dead-code deletion.

No diff change, so the approval still applies to the same tree.