[
https://issues.apache.org/jira/browse/HDDS-17356?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17356:
------------------------------
Attachment: TestBugMC3ContainerCacheReopenAfterErrorClose.java
> Data scan marks a container UNHEALTHY when its DB reopen fails on the lock
> still held after a RocksDB error close
> -----------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17356
> URL: https://issues.apache.org/jira/browse/HDDS-17356
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 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: Minor
> Attachments:
> MC-3-treat-db-lock-held-by-previous-instance-as-transient-scan-error.patch,
> TestBugMC3ContainerCacheReopenAfterErrorClose.java
>
>
> h3. Mechanism
> When a read or write on a container DB fails with {{Corruption}} or
> {{IOError}}, {{RocksDatabase.closeOnError}} marks the DB closed and starts a
> {{DBCloser}} thread. That thread frees the native DB, and releases its LOCK
> file, only when the last reference is gone; since HDDS-14800 an open iterator
> is such a reference. {{ContainerCache.getDB}} sees the closed store, removes
> it ({{removeDB}} calls {{ReferenceCountedDB.cleanup}} and {{stop}}, whose
> {{RocksDatabase.close}} returns at once because the close is already in
> progress) and reopens the same path. RocksDB rejects the open with "lock hold
> by current process" while the old instance still exists.
> {{BlockUtils.getDB}} turns that into a {{StorageContainerException}} with
> {{UNABLE_TO_READ_METADATA_DB}} and drops the cause. A data scan that gets
> this exception records {{INACCESSIBLE_DB}}, and {{ContainerScanHelper}}
> cannot recognise it as transient (the check added by HDDS-15150 walks the
> cause chain), so it marks the container UNHEALTHY and triggers a volume scan.
> h3. Trigger
> # A schema V1 or V2 container DB is open in {{ContainerCache}} and a thread
> iterates it (for example the container data scanner, which keeps its block
> iterator open while it scans a block).
> # Another operation on the same DB fails with {{Corruption}} or {{IOError}}
> (I used real corruption of the SST files).
> # A data scan of that container runs before the iterator is closed. This is
> likely: {{HddsDispatcher}} queues an on demand data scan after every failed
> request, including the original Corruption or IOError and each failed request
> in the window (subject to the minimum scan gap).
> Schema V3 is not affected: {{DatanodeStoreCache}} does not reopen a closed
> store.
> h3. Impact
> * The data scan marks a replica UNHEALTHY although the reopen failed only
> because the old instance had not been freed yet; SCM then replaces the
> replica. After a Corruption the mark may be justified anyway, after a
> transient IOError it is not. Only data scans open the DB; the metadata scan
> does not.
> * Client requests for the container fail with {{UNABLE_TO_READ_METADATA_DB}}
> until the iterator on the old instance is closed, and each schedules a volume
> check. The volume check does not open container DBs, so it does not fail a
> healthy volume. After the iterator is closed, {{getDB}} succeeds again, also
> on the same corrupted files. Clients retry on another replica.
> h3. Reproduction
> [^TestBugMC3ContainerCacheReopenAfterErrorClose.java]
> ({{hadoop-hdds/container-service}}) builds a schema V2 container DB, opens it
> through {{BlockUtils.getDB}} with an iterator, overwrites the start of each
> SST file, and reads another table, which fails with Corruption. A second
> {{BlockUtils.getDB}} then fails with {{UNABLE_TO_READ_METADATA_DB}} ("lock
> hold by current process" in the log). That exception is given to
> {{ContainerScanHelper.scanData}} the way {{KeyValueContainerCheck.scanData}}
> reports it. After the iterator is closed, the same {{getDB}} succeeds. The
> test asserts the correct behaviour, so on unfixed code it fails at its last
> assertion: the container was marked UNHEALTHY. With the patch it passes.
> h3. Patch
> [^MC-3-treat-db-lock-held-by-previous-instance-as-transient-scan-error.patch],
> against bf3829e6c8ac69ae17585585703f91426dcdf6b3. It also applies to master
> at 4fb68e96831. The open pull request for HDDS-17261 changes adjacent lines
> in {{BlockUtils.getDB}} and
> {{ContainerScanHelper.handleUnhealthyScanResult}}, so whichever lands second
> needs a small rebase.
> {{BlockUtils.getDB}} keeps the original exception as the cause, and
> {{ScanTransientIOUtil}} also treats "lock hold by current process" in the
> cause chain as transient ({{scanErrorsAreOnlyTooManyOpenFiles}} becomes
> {{scanErrorsAreOnlyTransient}}). A data scan in the window is then logged as
> incomplete and retried on a later pass instead of marking the container
> UNHEALTHY. The match is narrow: a LOCK held by another process is still an
> error. Requests in the window still fail transiently as described above;
> making them wait for the old instance is not proposed, because
> {{ContainerCache}} closes it under its cache wide lock and the holder can be
> a scanner that keeps its iterator for a whole block. As a side effect of
> keeping the cause, the HDDS-15150 "too many open files" check also works when
> the DB open itself fails on file descriptor exhaustion (from the code, not
> run). The two scan log messages that named only file descriptor exhaustion
> are reworded.
> Covered by {{testGetDBFailsWithLockCauseAfterErrorCloseWithOpenIterator}} in
> the existing {{TestContainerCache}} (real window, the failure must carry the
> RocksDB lock error, and {{getDB}} succeeds after the iterator is closed),
> {{testDataScanOnlyDbLockHeldByPreviousInstanceDoesNotMarkUnhealthy}} in
> {{TestBackgroundContainerDataScanner}}, and two cases in
> {{TestScanTransientIOUtil}} (lock held by this process is transient, lock
> held by another process is not). The first two fail without the change. With
> it the container cache, scanner, container check, dispatcher, handler,
> container and volume suites (420 tests) pass, checkstyle is clean.
> Found by TLA+ model checking and code review of the RocksDB handle close and
> shutdown paths 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]
