Fix CI state leakage and CC request lifetime races

[thweetkomputer]

Context

This PR addresses the two issues found while debugging the failing main CI run 29401455092.

1. The Tcl phases reused a live server that could still contain data from an earlier phase. That made scripting.tcl see stale keyspace state instead of a clean Redis database.
2. The crash was a real data_substrate lifetime race in stack-owned CC requests. The submodule update points at Fix stack CC request lifetime races tx_service#529.

The CC crash root cause is that CcShard::ProcessRequests() still owns a raw request pointer after Execute() signals the result waiter. For stack-owned requests, the waiter can reuse or destroy the stack object before ProcessRequests() gets back to its final req->Free() call.

Behavior before and after

Before:

• Some single-node and cluster Tcl phases ran without a flushall after startup, so state could leak across phases.
• Stack KickoutCcEntryCc request owners waited only for logical completion, not for the CC scheduler's final access to the request object.
• Stack range upload error paths could wake their waiter and still ask the scheduler to free/recycle the request.

After:

• The CI script flushes Redis data on the active single-node or cluster ports before each Tcl phase covered by this script.
• Stack KickoutCcEntryCc callers wait until scheduler-side Free() has completed before resetting the stack request.
• Stack range upload error/abort paths complete their waiter without returning true to the scheduler or calling Free().

Compatibility: runtime behavior should be unchanged outside the cleanup boundary and the corrected stack-request lifetime paths.

Implementation

• Added flush_redis_data() in .github/scripts/common.sh and call it before the affected single-node and cluster Tcl test phases.
• Updated data_substrate from f7e62c5 to 2ef3762:
  • LocalCcShards::PurgeDeletedData() and KickoutDataForTest() now use the existing Use() / InUse() request flag as a stack-object lifetime fence.
  • UploadRangeSlicesCc and UploadBatchSlicesCc stack-owned error paths return false / avoid Free() after waking their waiter.
  • Concurrency-control docs now spell out the stack-owned versus pool-owned request lifetime rule.

Design decisions and alternatives

The CC fix is local to the unsafe stack-owned requests rather than changing the generic scheduler contract. It reuses the existing request in_use_ flag and waits outside Execute() with bthread_usleep() backoff, so it does not add blocking or yield behavior to the CC execution path.

The CI cleanup is centralized in one shell helper so each phase fails fast if flushall cannot be applied to the expected port set.

Test plan

• Unit/TCL tests
• Integration or manual validation
• Formatting/build checks
• Compatibility or performance validation, when relevant

Commands and results:

git -C data_substrate diff --check
# passed

git diff --check
# passed

cmake --build bld-verify --target txservice -j4
# passed: [100%] Built target txservice

clang-format-18 --version
# failed: clang-format-18 not installed in this workspace

Full Tcl/CI tests were not run locally.

Risk assessment

The CI script now depends on redis-cli flushall succeeding on all listed ports before Tcl execution. That is intentional; a failed flush should fail the job rather than hiding stale state.

For the CC change, the main risk is a stack KickoutCcEntryCc caller waiting slightly longer after logical completion. The wait only covers the scheduler gap between result notification and ProcessRequests() calling Free().

This PR should remain draft until eloqdata/tx_service#529 is reviewed and landed or the submodule pointer is otherwise updated to the landed commit.

Rollback plan

Revert this PR. If only the CI cleanup needs rollback, revert the .github/scripts/common.sh changes while keeping the submodule update.

Reviewer guide

• .github/scripts/common.sh: flush_redis_data() and Tcl phase call sites.
• data_substrate submodule diff: Fix stack CC request lifetime races tx_service#529.

Follow-up work

Run the full GitHub Actions matrix after the submodule PR lands and this pointer is updated if needed.

Summary by CodeRabbit

• Tests
  • Improved test isolation by clearing Redis state before running both single-node and multi-port Redis scenarios.
  • Added reusable Redis flush helper to standardize cleanup across test workflows.
  • Updated workflows to stop immediately if Redis cleanup fails, preventing cascading failures.
  • Adjusted external-server test setup to begin from a clean Redis data state.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cefa7396-21a4-42a7-8e10-7989cd28ebdc

📥 Commits

Reviewing files that changed from the base of the PR and between 2b10849 and 9a65d4c.

📒 Files selected for processing (1)

• data_substrate

---

Walkthrough

Redis flushing is centralized for single-node and cluster test workflows, failures now stop those workflows, external-server tests clear Redis during setup, and the data_substrate subproject pointer is updated.

Changes

Redis reset behavior

Layer / File(s)	Summary
Flush helper and workflow integration .github/scripts/common.sh	Adds flush_redis_data with port logging, default-port handling, per-port flushing, and failure propagation across single-node and cluster workflows.
External server initialization reset tests/support/server.tcl	Enables r flushall during external-server test initialization.

Substrate revision

Layer / File(s)	Summary
Subproject pointer update data_substrate	Updates the recorded subproject commit from f7e62c5… to d08da29….

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

Possibly related PRs

• eloqdata/eloqkv#333: Updates the same data_substrate submodule pointer.
• eloqdata/eloqkv#536: Changes the pinned data_substrate commit.
• eloqdata/eloqkv#537: Modifies the data_substrate submodule reference.

Suggested labels: trigger-ci

Suggested reviewers: liunyl

Poem

I’m a rabbit with a broom,
Flushing Redis room by room.
Ports are logged, failures flee,
Tests start fresh as fresh can be.
A substrate pointer hops anew—
Carrot cheers for this review! 🐇

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title concisely describes the main changes: CI state cleanup and CC request lifetime fixes.
Description check	✅ Passed	The description follows the template and covers context, behavior, implementation, testing, risks, rollback, and follow-up work.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ci-flush-free-investigation

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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