You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "aglinxinyuan (via GitHub)" <gi...@apache.org> on 2026/08/09 11:56:32 UTC
[PR] chore(amber): remove the unused EmptyReplayLogger [texera]
aglinxinyuan opened a new pull request, #7452:
URL: https://github.com/apache/texera/pull/7452
### What changes were proposed in this PR?
Deletes `EmptyReplayLogger`, a no-op `ReplayLogger` that the log-replay factory no longer reaches. Pure deletion, no behaviour change: **−171 lines**.
| Removed | Lines | Why it is dead |
| --- | ---: | --- |
| `engine/architecture/logreplay/EmptyReplayLogger.scala` | 41 | never instantiated |
| `engine/architecture/logreplay/EmptyReplayLoggerSpec.scala` | 130 | covers only the above |
The null-object role moved up a layer to `EmptyReplayLogManagerImpl`, which implements the no-ops inline:
```
ReplayLogManager.scala:51 -> EmptyReplayLogManagerImpl (inline no-ops, live)
ReplayLogManager.scala:109 -> new ReplayLoggerImpl() (the only ReplayLogger built in main)
EmptyReplayLogger (never instantiated)
```
No configuration selects a logger class by name, so there is no path that revives it.
> Reviewer note: the abstract base `ReplayLogger` is **not** touched — `ReplayLoggerImpl` extends it and is unaffected. `EmptyReplayLogger` acquired unit tests during the 2026 coverage work, which is why it currently looks live; the spec covers this class and nothing else.
Split out of #7447, which originally bundled four unrelated removals.
### 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
```
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore(amber): remove the unused EmptyReplayLogger [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7452:
URL: https://github.com/apache/texera/pull/7452#issuecomment-5231390777
<!-- texera-reviewer-suggestion -->
### 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore(amber): remove the unused EmptyReplayLogger [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on PR #7452:
URL: https://github.com/apache/texera/pull/7452#issuecomment-5299907513
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` (#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 #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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore(amber): remove the unused EmptyReplayLogger [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan merged PR #7452:
URL: https://github.com/apache/texera/pull/7452
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore(amber): remove the unused EmptyReplayLogger [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #7452:
URL: https://github.com/apache/texera/pull/7452#issuecomment-5234682561
I also think this looks unused. @shengquan-ni can you help confirm?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore(amber): remove the unused EmptyReplayLogger [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #7452:
URL: https://github.com/apache/texera/pull/7452#issuecomment-5231438757
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/7452?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 85.32%. Comparing base ([`e03d971`](https://app.codecov.io/gh/apache/texera/commit/e03d97108ddd47befc78a87f467f781dd50a56e6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`ac3a565`](https://app.codecov.io/gh/apache/texera/commit/ac3a56560c9a45c3ba6f50ae14a169f1712d30aa?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:white_check_mark: All tests successful. No failed tests found.
<details><summary>Additional details and impacted files</summary>
```diff
@@ 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](https://app.codecov.io/gh/apache/texera/pull/7452/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/7452/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.00% <ø> (ø)` | | Carriedforward from [e03d971](https://app.codecov.io/gh/apache/texera/commit/e03d97108ddd47befc78a87f467f781dd50a56e6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/7452/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `85.50% <ø> (ø)` | | Carriedforward from [e03d971](https://app.codecov.io/gh/apache/texera/commit/e03d97108ddd47befc78a87f467f781dd50a56e6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/7452/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `80.88% <ø> (-0.01%)` | :arrow_down: | |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/7452/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `50.72% <ø> (ø)` | | Carriedforward from [e03d971](https://app.codecov.io/gh/apache/texera/commit/e03d97108ddd47befc78a87f467f781dd50a56e6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/7452/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `65.97% <ø> (ø)` | | Carriedforward from [e03d971](https://app.codecov.io/gh/apache/texera/commit/e03d97108ddd47befc78a87f467f781dd50a56e6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/7452/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `69.05% <ø> (ø)` | | Carriedforward from [e03d971](https://app.codecov.io/gh/apache/texera/commit/e03d97108ddd47befc78a87f467f781dd50a56e6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/7452/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `87.85% <ø> (ø)` | | Carriedforward from [e03d971](https://app.codecov.io/gh/apache/texera/commit/e03d97108ddd47befc78a87f467f781dd50a56e6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/7452/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.89% <ø> (ø)` | | Carriedforward from [e03d971](https://app.codecov.io/gh/apache/texera/commit/e03d97108ddd47befc78a87f467f781dd50a56e6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/7452/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `97.51% <ø> (ø)` | | Carriedforward from [e03d971](https://app.codecov.io/gh/apache/texera/commit/e03d97108ddd47befc78a87f467f781dd50a56e6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/7452/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `26.31% <ø> (ø)` | | Carriedforward from [e03d971](https://app.codecov.io/gh/apache/texera/commit/e03d97108ddd47befc78a87f467f781dd50a56e6?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/7452?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore(amber): remove the unused EmptyReplayLogger [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7452:
URL: https://github.com/apache/texera/pull/7452#issuecomment-5231413583
<!-- texera-benchmarks-comment -->
## ⚠️ 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](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/31312035601)
| | 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% |
<details><summary>Baseline details</summary>
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% |
</details>
<details><summary>Raw CSV</summary>
```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
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org