yangxianjungree commented on issue #4705: URL: https://github.com/apache/bookkeeper/issues/4705#issuecomment-5435388693
We hit a related shutdown crash with `entryLocationCompactionInterval` enabled, but it is not the same failure as the stack in this issue, and [#4706](https://github.com/apache/bookkeeper/pull/4706) does not cover it. The stack pasted here is a caught `RocksDBException: Database shutdown` from `entryLocationCompact` → `compactRange`. In our case the JVM actually SIGSEGV'd. The crashing thread was always `db-storage-cleanup-*` (the single-thread `cleanupExecutor`), and the Java frames were the **checkpoint cleanup** task, not compact: ``` SingleDirectoryDbLedgerStorage.lambda$checkpoint$9 → EntryLocationIndex.removeOffsetFromDeletedLedgers → RocksDBBatch.flush → RocksDB.write0 // locations DB // or → LedgerMetadataIndex.removeDeletedLedgers → KeyValueStorageRocksDB.delete → rocksdb_delete_helper // ledgers DB ``` What happens: 1. `entryLocationCompact()` submits a sync `compactRange()` onto `cleanupExecutor` and occupies that thread for a long time. 2. `checkpoint()` / `flush()` `finally` submits a **different** task onto the same executor: `removeOffsetFromDeletedLedgers()` then `removeDeletedLedgers()`. That task sits in the `ThreadPoolExecutor` queue behind compact. 3. `SingleDirectoryDbLedgerStorage.shutdown()` does `cleanupExecutor.shutdown()` + `awaitTermination(1, SECONDS)` (return value unused), then `ledgerIndex.close()` and `entryLocationIndex.close()`. 4. After compact's Java task returns, the queued checkpoint cleanup runs `flush()`/`delete()` with no `closedLock` on those paths (PR #4581 only guards `count()`). That is a native write/delete vs close race. Compact is the window amplifier; it is not the crashing frame. Why #4706 does not fix this class of crash: - `EntryLocationIndex.close()` only spins on the `compacting` flag, then closes `locationsDb`. It does not drain `cleanupExecutor`. When compact's `finally` sets `compacting=false`, close can proceed while the checkpoint cleanup task is still queued or about to run. - `ledgerIndex.close()` still happens first and is not covered by that wait. A queued `removeDeletedLedgers()` can hit an already-closed ledgers DB (we have an hs_err on `DBImpl::Delete` / `FailIfCfHasTs`). - `db.cancelAllBackgroundWork(true)` does not cancel an in-flight sync `compactRange()`, and it does not fence the later `WriteBatch.flush` / `delete` from checkpoint cleanup. A fix for this issue's `compactRange` vs close exception is still useful, but it is not sufficient for the queued-checkpoint-cleanup SIGSEGV. That needs a stopping fence plus draining (or skipping) the executor **before** either RocksDB is closed, and/or the same close-guard on `flush()`/`delete()` that `count()` already has. Happy to add more stacks if useful. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
