Skip to content

fix(rocksdb): retain Topling OLAP handles - #3299

Open
contrueCT wants to merge 4 commits into
apache:masterfrom
contrueCT:fix/topling-olap-clear
Open

contrueCT wants to merge 4 commits into
apache:masterfrom
contrueCT:fix/topling-olap-clear

Conversation

@contrueCT

@contrueCT contrueCT commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Purpose of the PR

Topling's standalone OLAP property clear still drops and recreates the same column family. The previously reported real-JNI reproduction completes the clear task, then aborts during normal shutdown. This change clears the selected table while retaining its CF handle.

Addresses the OLAP clear/close item already tracked in #3242 and the original review. The release checklist stays open. Existing Topling and recovery PRs were checked; the merged runtime integration and snapshot-recovery fix #3293 do not cover this clear path.

Main Changes

  • Clear the selected OLAP table in its owning database under the store write lock.
  • Retain Topling CF handles and discard the clearing thread's pending batch before committing the clear; leave other threads' leases untouched.
  • Preserve the standard RocksDB drop/create path.
  • Retry transient Arthas session-probe failures during CLI cleanup after an explicit stop. Retain credential and PID validation and require connection refusal before reporting that the listener is closed.
  • Adapt OLAP test fixtures to the shared AtomicReference owner introduced by merged fix(rocksdb): make snapshot restore retryable #3293.

Verifying these changes

  • Trivial rework / code cleanup without any test coverage. (No Need)
  • RocksDBSessionsTest: 30 tests pass on Java 17, with no failures/errors/skips. Coverage includes retained handle identity, recreation on each repeated standard clear, pending writes outside the stored range and in an empty table, independent OLAP routing, other-table preservation, reuse and close/reopen reads. A failed-commit test verifies rollback, retained handles, balanced session references and successful retry. A second-thread test verifies that a batch staged before clear can repopulate the table when committed afterward.
  • Isolated negative controls fail as expected when the second standard clear skips handle recreation or when OLAP clear resets other threads' pending batches.
  • PropertyKeyCoreTest: 24 tests pass on Java 17 with the RocksDB backend, including OLAP clear/remove integration coverage.
  • Arthas native explicit-stop regression: an injected cleanup disconnect fails on the original script and passes with bounded retries. Attachment, stop and the subsequent port check use the bundled Arthas 4.1.5 runtime.
  • Whole-repository Java 17 clean compilation, editorconfig formatting and git diff --check pass.

The full Linux Arthas CLI suite, including the shutdown-race regression, passed on this head in CI run 41295617424. Other jobs in that run are still in progress. The local macOS run stops at the IPv6 closed-port check: the bundled client returns exit 100 without the connection-refused diagnostic. The remaining macOS native CLI scenarios pass in a local copy excluding that explicit IPv6 bind case; this does not establish a complete native CLI pass.

These tests run the Topling Java branch with the real standard RocksDB JNI. The same handle-preservation regression fails against the original master adapter. Actual Topling JNI and its reported shutdown abort were not rerun on this macOS arm64 host; current-head Linux native qualification remains necessary.

Does this PR potentially affect the following parts?

  • Dependencies
  • Modify configurations
  • The public API
  • Other affects: standalone Topling OLAP clear and native handle lifecycle
  • Nope

Documentation Status

  • Doc - TODO
  • Doc - Done: docs/storage-lifecycle.md documents OLAP clear, handle retention and pending-write handling, including later commits from concurrent writers and the difference from graph truncate.
  • Doc - No Need

Conceptual overview

Topling OLAP clear 保留 CF handle,标准 RocksDB 仍走 drop/create,实际 Topling JNI 验收仍需完成。

Conceptual illustration; not test or release-acceptance evidence.

Topling OLAP clear retains column-family handle with native acceptance boundary

English conceptual overview

Topling OLAP clear retains the selected table column-family handle while standard RocksDB retains its drop-and-create path

Concept illustration of the Topling OLAP clear path: reset the clearing thread's pending batch, clear the selected table in its owning database, and retain the CF handle. Standard RocksDB retains drop/create; actual Topling JNI qualification remains required.

@codecov

codecov Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.05%. Comparing base (608cb1d) to head (1cabcdf).
⚠️ Report is 27 commits behind head on master.

Files with missing lines Patch % Lines
.../hugegraph/backend/store/rocksdb/RocksDBStore.java 87.50% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3299      +/-   ##
============================================
+ Coverage     57.27%   58.05%   +0.78%     
+ Complexity     6577        3    -6574     
============================================
  Files           776      508     -268     
  Lines         68881    49257   -19624     
  Branches       9431     7320    -2111     
============================================
- Hits          39449    28595   -10854     
+ Misses        25804    17949    -7855     
+ Partials       3628     2713     -915     
Flag Coverage Δ
commons ?
hstore ?
pd ?
server_hbase ?
server_memory 56.54% <87.50%> (+0.56%) ⬆️
server_rocksdb 43.13% <50.00%> (+0.47%) ⬆️
store ?

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@contrueCT
contrueCT marked this pull request as draft October 9, 2026 13:52
@contrueCT
contrueCT force-pushed the fix/topling-olap-clear branch from a440bdc to 6a068b9 Compare October 10, 2026 04:44
@contrueCT
contrueCT marked this pull request as ready for review October 10, 2026 04:50
Clear OLAP data without dropping its column family.
Route the clear to the owning database under the store lock.
Discard only the clearing threads pending batch.
Cover handle identity, other tables, reuse and reopen.
@contrueCT
contrueCT force-pushed the fix/topling-olap-clear branch from 6a068b9 to 34cc360 Compare October 10, 2026 11:58

@bitflicker64 bitflicker64 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: no. Summary: The Topling branch clears the selected OLAP table through the existing RocksDBSessions.clearTables path under the store write lock, with balanced useSession/close and the standard drop/create path unchanged; no correctness defect found in the Java change, two small test-coverage notes inline. Real Topling JNI shutdown behaviour was not exercised by this review or by CI. Evidence: read the full diff at 34cc360 against merge base 87a06a9 plus RocksDBStore, RocksDBSessions.clearTables, BackendSessionPool, RocksDBStdSessions.StdSession and the callers (GraphTransaction.clearOlapPk, OlapPropertyKeyClearJob); RocksDBSstStore does not inherit clearOlapTable so the SST backend is unaffected; CI job build-server / server_memory (Java 17) ran UnitTestSuite at this head: 999 tests, 0 failures, 0 errors.

Compare each recreated handle with the previous clear.
Pin pending writes committed after another worker clears.
Document concurrent writes after OLAP property clear.
Retry transient cleanup disconnects after explicit stop.
Keep credential and target PID checks before stopping an agent.
Reproduce the race with a one-shot fault around the native CLI.
Cover OLAP clear rollback and session leases after failed commits.

@bitflicker64 bitflicker64 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: no. Score: 9/10. Summary: The Topling branch of clearOlapTable clears only the selected OLAP table through RocksDBSessions.clearTables under the store write lock, resets the clearing thread's batch and keeps its CF handle, while the standard path keeps drop/create; the earlier handle-refresh, second-thread and AtomicReference fixture notes are addressed, and the Arthas probe retry keeps the credential and PID checks.

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.

3 participants