Skip to content

[ISSUE #10334] Make native CqCompactionFilter shim self-contained without rocksdbjni dependency#10371

Merged
lollipopjin merged 2 commits into
apache:developfrom
lizhimins:fix/cq-compaction-filter-double-load
May 25, 2026
Merged

[ISSUE #10334] Make native CqCompactionFilter shim self-contained without rocksdbjni dependency#10371
lollipopjin merged 2 commits into
apache:developfrom
lizhimins:fix/cq-compaction-filter-double-load

Conversation

@lizhimins
Copy link
Copy Markdown
Member

Summary

  • Make the native CqCompactionFilter shim fully self-contained by including stub vtable implementations for all platforms, eliminating the DT_NEEDED dependency on librocksdbjni
  • Remove ensureRocksDBNativeLoaded() from CqCompactionFilterJni — the shim no longer needs rocksdbjni loaded first
  • Add synchronized lifecycle management (FILTER_LOCK) with proper destroyNativeFilter() called in preShutdown()
  • Recompile all platform binaries (x86_64, aarch64, macOS arm64, Windows x64) with JDK 8 headers

Motivation

The previous shim had a DT_NEEDED on librocksdbjni-linux64.so. Since rocksdbjni has no SONAME, dlopen uses pathname-based dedup — different temp extraction paths produce different inodes, causing a second copy of rocksdbjni to be loaded into the process. Two copies of RocksDB global state (Env, thread pools) coexist, leading to intermittent SIGSEGV.

Changes

File Change
CqCompactionFilterJni.java Remove ensureRocksDBNativeLoaded(); add FILTER_LOCK + destroyNativeFilter(); simplify loadNativeShim()
ConsumeQueueRocksDBStorage.java Call CqCompactionFilterJni.destroyNativeFilter() in preShutdown()
cq_compaction_filter.cpp Remove #ifdef _WIN32 guard on stubs — now unconditional for all platforms
libcq_compaction_filter.so Rebuilt x86_64 (self-contained, no DT_NEEDED on rocksdbjni)
libcq_compaction_filter_aarch64.so Rebuilt aarch64 (self-contained)
libcq_compaction_filter.dylib Rebuilt macOS arm64 (self-contained)
cq_compaction_filter.dll Rebuilt Windows x64 with MSVC (self-contained)
docs/en/Native_RocksDB.md Updated build instructions

Verification

# Verify no rocksdbjni dependency
objdump -p libcq_compaction_filter.so | grep NEEDED
# Shows only: libstdc++, libm, libgcc_s, libc

# CqCompactionFilterJniTest passed 5x on each platform:
# - Linux x86_64 (JDK 8): 5/5 PASS
# - Linux aarch64 (JDK 8): 5/5 PASS
# - Windows x64 (JDK 8): 5/5 PASS

Test plan

  • CqCompactionFilterJniTest passes on Linux x86_64, aarch64, Windows x64, macOS arm64
  • Full store module test suite passes (no regressions from this change)
  • Repeated execution (5x per platform) shows no flakiness
  • objdump -p confirms no DT_NEEDED on librocksdbjni

🤖 Generated with Claude Code

lizhimins and others added 2 commits May 23, 2026 16:47
…ed without rocksdbjni dependency

Remove DT_NEEDED on librocksdbjni by including stub vtable implementations
for all platforms. Simplify CqCompactionFilterJni loader and add synchronized
lifecycle management with proper destroy in preShutdown().

Recompile all platform binaries (x86_64, aarch64, Windows) with JDK 8 headers.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ing JDK 8 headers

Recompiled libcq_compaction_filter.dylib on macOS arm64 with the same
self-contained stub approach — no dependency on librocksdbjni. Built
with JDK 8 headers for maximum compatibility (verified on JDK 8 and 21).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.46154% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.86%. Comparing base (54708be) to head (bba9a8a).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
.../rocketmq/store/rocksdb/CqCompactionFilterJni.java 88.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             develop   #10371      +/-   ##
=============================================
- Coverage      48.95%   48.86%   -0.10%     
+ Complexity     13472    13450      -22     
=============================================
  Files           1376     1376              
  Lines         100546   100537       -9     
  Branches       12984    12983       -1     
=============================================
- Hits           49225    49129      -96     
- Misses         45319    45391      +72     
- Partials        6002     6017      +15     

☔ View full report in Codecov by Sentry.
📢 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.

Copy link
Copy Markdown
Contributor

@lollipopjin lollipopjin left a comment

Choose a reason for hiding this comment

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

LGTM

@lollipopjin lollipopjin merged commit 7e5d22d into apache:develop May 25, 2026
10 of 16 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.

3 participants