Siyao Meng created HDDS-17355:
---------------------------------
Summary: 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
Reporter: Siyao Meng
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]
