[jira] [Updated] (HDDS-17356) Data scan marks a container UNHEALTHY when its DB reopen fails on the lock still held after a RocksDB error close

["Siyao Meng (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/HDDS-17356?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Siyao Meng updated HDDS-17356:
------------------------------
    Affects Version/s: 2.2.1
                       2.0.0
                       2.1.2
                       2.1.0
                       1.4.1
                       2.1.1
                       2.2.0

> 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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org