Skip to content

test(meta): check publication session at its Applied receipt - #316

Merged
thweetkomputer merged 1 commit into
eloqdata:mainfrom
thweetkomputer:fix/meta-publication-ci
Oct 12, 2026
Merged

thweetkomputer merged 1 commit into
eloqdata:mainfrom
thweetkomputer:fix/meta-publication-ci

Conversation

@thweetkomputer

@thweetkomputer thweetkomputer commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Context

meta_integration.gate_data_publication counts accepted connections until the cluster returns to READY. A replacement can finish on its original connection, then a delayed lease Grant can expire and correctly trigger reauthentication before that readiness check. The test reports this later reconnect as a publication failure.

Based on current main (bdb0d2e12), following the merge of #314.

Behavior before and after

Wait for the first FullStateApplied after the obsolete transfer's TransferAbort, and require that receipt to come from the connection which carried the held chunk. A replacement completed through a new connection still fails. Lease recovery after the replacement completes no longer causes this assertion to fail.

Implementation

Change only the existing integration test. Preserve the obsolete-object TransferEnd rejection, final READY check, and proxy-error checks.

Design decisions and alternatives

Correlate the actual completion receipt with its socket pair. This gives the assertion a protocol-defined endpoint instead of widening timing budgets or permitting reconnection during replacement.

Documentation and comments

Added a nearby comment explaining the distinction between Applied completion and subsequent lease recovery. No architectural claims or production behavior change.

Test plan

cmake --build build_debug --target lavik-meta lavik-ctl --parallel 6
ctest --test-dir build_debug -R '^meta_integration.gate_data_publication$' --output-on-failure --repeat until-fail:30
pre-commit run --files tests/meta_integration/gate_data_publication.py
git diff --check

Build, all 30 consecutive integration runs, formatting, and diff checks passed.

Controlled proxy probes also checked both sides of the assertion:

  • Delay one 250 ms lease Grant by 300 ms and delay the post-transfer READY query by 400 ms. Data logs lease grant arrived after its deadline; the replacement's Applied receipt precedes the reconnect on the original connection. The old test fails its reconnect-count assertion; the updated test passes.
  • Close the connection immediately after forwarding TransferAbort, forcing replacement bootstrap on a new connection. The updated test still fails its session assertion.

The original CI failure did not retain server logs, so its exact reconnect cause cannot be recovered. These probes demonstrate the assertion's false-positive case and confirm that the required in-session replacement behavior remains checked.

Risk assessment

Limited to test synchronization and assertion scope. Production code and lease-expiry enforcement are unchanged.

Rollback plan

Revert this test-only commit.

Reviewer guide

Review the Applied-receipt wait and socket-pair comparison in gate_data_publication.py.

Follow-up work

CI validation on both architectures.

Summary by CodeRabbit

  • Tests
    • Updated integration checks to verify that state is applied after an obsolete transfer is aborted and remains associated with the original transfer connection.
    • Retained checks that the system reaches readiness and that the superseded transfer does not complete.

@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 65239d4b-6542-4d94-9517-0735f00c4477

📥 Commits

Reviewing files that changed from the base of the PR and between bdb0d2e and 18ee258.


📒 Files selected for processing (1)
  • tests/meta_integration/gate_data_publication.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.



Walkthrough

The integration test now checks that a FullStateApplied event after an obsolete transfer abort uses the original transfer chunk’s connection. It retains the READY wait and the assertion that the obsolete transfer did not reach TransferEnd.

Changes

Data publication test

Layer / File(s) Summary
Verify connection after abort
tests/meta_integration/gate_data_publication.py
The test replaces its aggregate accepted-connection count check with a check that a later FullStateApplied event uses the original chunk connection. The READY wait and TransferEnd assertion remain.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Other

Suggested reviewers: liunyl

Merge Risk: ⚪ Minimal · up to 18ee2

The test checks that the replacement is applied on the original connection while retaining its other publication checks. No merge-blocking issue was established.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check Passed The description includes all required template sections and clearly explains the test change, implementation, validation commands, risks, rollback, and follow-up work.
Title check Passed The title is concise and accurately identifies the integration-test change: checking the publication session at the Applied receipt.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks the state anew
The chunk connection carries through
The old transfer stops its flight
READY comes and marks it right
No TransferEnd for stale flow
The test confirms what paths now show

Comment @coderabbitai help to get the list of available commands.

@thweetkomputer
thweetkomputer merged commit 8028ec3 into eloqdata:main Oct 12, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant