[
https://issues.apache.org/jira/browse/HDDS-17355?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
ASF GitHub Bot updated HDDS-17355:
----------------------------------
Labels: pull-request-available (was: )
> Datanode stop aborts halfway when a schema V1/V2 container DB is still in
> use, ContainerCache.shutdownCache throws IllegalArgumentException
> -------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17355
> URL: https://issues.apache.org/jira/browse/HDDS-17355
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 1.3.0, 1.4.0, 2.0.0, 1.4.1, 2.1.0, 2.2.0, 2.1.1, 2.1.2,
> 2.2.1
> Reporter: Siyao Meng
> Priority: Major
> Labels: pull-request-available
> Attachments: CR-8-skip-container-dbs-in-use-on-cache-shutdown.patch,
> TestBugCR8ShutdownCacheAbortsStop.java
>
>
> h3. Mechanism
> {{ContainerCache.shutdownCache}} runs
> {{Preconditions.checkArgument(cleanupDb(db), "refCount: %s", ...)}} for every
> cached schema V1/V2 container DB, and {{ReferenceCountedDB.cleanup}} returns
> false while the DB is referenced. Since HDDS-6540 this is on the datanode
> stop path: {{OzoneContainer.stop}} calls {{KeyValueHandler.stop}}, then
> {{BlockManagerImpl.shutdown}} and {{BlockUtils.shutdownCache}}. Nothing on
> the way out catches the {{IllegalArgumentException}}
> ({{DatanodeStateMachine.stopDaemon}} only catches {{IOException}}), so the
> rest of the shutdown is skipped: the remaining cached V1/V2 DBs,
> {{DatanodeStoreCache.shutdownCache}} (the schema V3 per volume DBs), the
> volume sets and the other services after the handlers in
> {{OzoneContainer.stop}}, and the remainder of {{DatanodeStateMachine.close}}
> and {{HddsDatanodeService.stop}}. Nothing retries the skipped steps. The
> message is {{refCount: 1}} at this commit and {{refCount: [1]}} in released
> versions, whose format string has no placeholder.
> h3. Trigger
> A datanode holding schema V1/V2 containers (upgraded from a release before
> schema V3, or with schema V3 disabled) stops while a thread holds a handle
> from {{BlockUtils.getDB}}. {{OzoneContainer.stop}} stops the handlers before
> {{blockDeletingService}}, so a running {{BlockDeletingTask}} is a natural
> holder. HDDS-16131 hit the same exception in a test teardown that called
> {{BlockUtils.shutdownCache}} while a {{BlockDeletingTask}} still held a DB,
> and was fixed in the test only. {{DeleteBlocksCommandHandler}} workers are
> another holder, since their pool is stopped after {{container.stop()}} (code
> reading, not run).
> h3. Impact
> * On a normal SIGTERM the shutdown hook logs the exception and the JVM exits,
> so the skipped DBs are closed by process exit and RocksDB recovers on the
> next start.
> * When the datanode stops itself through
> {{HddsDatanodeService.terminateDatanode}} ({{stop(); terminate(1);}}, used
> when Ratis closes a container state machine in {{ContainerStateMachine}}, for
> too many failed volumes, a critical state machine error, a failed certificate
> rollback and a failure to persist a new certificate ID in {{saveNewCertId}}),
> the exception escapes {{stop()}} and {{terminate(1)}} is not reached. Checked
> locally with a test that starts an {{HddsDatanodeService}} with a schema V2
> container, holds a {{BlockUtils.getDB}} handle and calls
> {{terminateDatanode}}: {{stop()}} throws {{IllegalArgumentException}},
> {{terminate(1)}} is not called, {{isStopped}} is true, and two non daemon
> threads (one of them Jetty's scheduler) stay alive, so the JVM does not exit.
> With the handle released, or with the patch, {{terminate(1)}} is called and
> no extra non daemon thread is left. The datanode is meant to exit there so
> that a supervisor restarts it; instead it stays up with its services stopped
> until someone restarts it by hand.
> h3. Reproduction
> PASS on unmodified source, deterministic.
> [^TestBugCR8ShutdownCacheAbortsStop.java] ({{hadoop-hdds/container-service}})
> starts a real {{OzoneContainer}} with schema V3 enabled, creates two schema
> V2 containers, holds a handle on one of them through {{BlockUtils.getDB}} and
> calls {{OzoneContainer.stop}} on the same thread. {{stop}} throws
> {{IllegalArgumentException: refCount: 1}}; afterwards the other V2 DB and the
> schema V3 per volume DB are still open and the data volume was not shut down.
> A second {{stop}} after the handle is released completes the shutdown. A
> passing test means the defect is present. The {{terminateDatanode}} check
> above is a separate local test, not attached.
> h3. Patch
> [^CR-8-skip-container-dbs-in-use-on-cache-shutdown.patch], against
> bf3829e6c8ac69ae17585585703f91426dcdf6b3. It also applies to master at
> 4fb68e96831.
> {{shutdownCache}} closes and removes every DB it can close and leaves a DB
> that is still referenced open and in the cache, with one WARN line naming it,
> instead of throwing. Closing it would make the user's next DB call fail and
> break the reference count contract. A skipped DB is closed by a later
> eviction, {{removeDB}} or {{shutdownCache}} once released, or by process exit
> on a real stop, which is what happens to every DB today after the exception.
> Nothing changes when no DB is referenced. Reordering {{OzoneContainer.stop}}
> to stop background services first would only narrow the window, so it is not
> part of the patch.
> Covered by {{testShutdownCacheKeepsDBInUseOpen}} in the existing
> {{TestContainerCache}}, which fails without the change with
> {{IllegalArgumentException: refCount: 1}}. With the change
> {{TestContainerCache}}, {{TestOzoneContainer}}, {{TestBlockDeletingService}},
> {{TestContainerPersistence}}, the schema one and two backwards compatibility
> suites, {{TestBlockManagerImpl}}, {{TestContainerReader}},
> {{TestDatanodeStoreCache}}, {{TestKeyValueBlockIterator}} and
> {{TestStaleRecoveringContainerScrubbingService}} pass (586 tests), and
> checkstyle is clean. With the patch the reproduction fails at its first
> assertion because {{stop}} no longer throws.
> Found by code review of the RocksDB handle close and shutdown paths, as part
> of the TLA+ verification effort under HDDS-15926, on commit
> bf3829e6c8ac69ae17585585703f91426dcdf6b3. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. The attached patch
> is a proposal for review. Generated with Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
