[
https://issues.apache.org/jira/browse/HDDS-17015?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17015:
------------------------------
Attachment: CR-11-use-concurrent-map-for-db-volume-store-paths.patch
> ConcurrentModificationException in DbVolume.closeAllDbStore leaves a failed
> DB volume active and stops the periodic disk check
> ------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17015
> URL: https://issues.apache.org/jira/browse/HDDS-17015
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Minor
> Attachments:
> CR-11-use-concurrent-map-for-db-volume-store-paths.patch,
> TestBugCR11DbVolumeMapRace.java
>
>
> h3. Mechanism
> {{DbVolume.hddsDbStorePathMap}} is a {{HashMap}}.
> {{HddsVolume.createDbStore}} adds to it through {{addHddsDbStorePath}} while
> SchemaV3 finalization holds the data volume set lock. {{DbVolume.failVolume}}
> iterates it in {{closeAllDbStore}} while the DB volume set lock is held. No
> lock is shared, so an add during the loop makes the iterator throw
> {{ConcurrentModificationException}}.
> The exception leaves {{MutableVolumeSet.failVolume}} before the volume is
> moved to the failed set, then {{handleVolumeFailures}}, {{checkAllVolumes}}
> and {{StorageVolumeChecker.checkAllVolumeSets}}, which only catches
> {{IOException}}. That method is the body of the {{scheduleWithFixedDelay}}
> task of the periodic disk check, and a scheduled task that throws is not run
> again.
> h3. Trigger
> A datanode with dedicated DB volumes ({{hdds.datanode.container.db.dir}})
> finalizes SchemaV3 while the periodic disk check fails one of its DB volumes,
> and a {{createDbStore}} adds a path to that DB volume during the close loop.
> Before finalization completes each new DB is closed right after creation, so
> the loop only does cache lookups and the window is very short.
> h3. Impact
> * The DB volume is FAILED but stays in the active DB volume set. The rest of
> the finalize loop can still place new DBs on it, and the DB load that ends
> the finalization ({{HddsVolumeUtil.loadAllHddsVolumeDbStore}}) maps its data
> volumes back to it and reopens their DBs there (by code reading).
> * The periodic disk check stops for all volume sets until the datanode
> restarts, so later disk failures are not detected by it. The exception is
> never logged, because nothing reads the scheduled future; the operator only
> sees the earlier log lines that mark the DB volume as failed. On demand
> checks after I/O errors still run.
> * Rare: needs a DB volume failure during the one-time finalization, and the
> window was not hit naturally.
> h3. Reproduction
> PASS on unmodified source. [^TestBugCR11DbVolumeMapRace.java]
> ({{hadoop-hdds/container-service}}, target path and Maven command in the
> class comment) registers a real DB volume set with a real
> {{StorageVolumeChecker}} and runs the real {{checkAllVolumeSets}} as a
> {{scheduleWithFixedDelay}} task. The disk check verdict is stubbed to report
> the DB volume as failed, and {{DatanodeStoreCache}} is mocked so that the
> first {{removeDB}} of the close loop registers one more DB path, as a
> concurrent {{createDbStore}} would. Output: {{periodic check runs=1
> dbVolumeState=FAILED stillInActiveSet=true failedSet=0}}, and the scheduled
> future completes with the {{ConcurrentModificationException}} from
> {{DbVolume.closeAllDbStore}} and is not run again. A passing test means the
> defect is present.
> h3. Patch
> [^CR-11-use-concurrent-map-for-db-volume-store-paths.patch], against
> cce1ca28944886218132f01c0e271c1fd0700078. It also applies to master at
> 2536fe9cbc3.
> The map becomes a {{ConcurrentHashMap}}. Its iterators do not throw on
> concurrent changes, so {{failVolume}} completes and the periodic task keeps
> running. Keys and values are never null, and the other uses ({{put}},
> {{clear}}, {{keySet}}, {{values}}) behave the same.
> Covered by {{testFailVolumeWhileDbStorePathIsAdded}} in the existing
> {{TestDbVolume}}, which adds a path from the first {{removeDB}} call of
> {{failVolume}} and asserts that the volume is failed and every earlier path
> was closed. Without the change it fails with
> {{ConcurrentModificationException}}. With it the DB volume, volume, volume
> set, checker and upgrade suites (83 tests) pass and checkstyle is clean. With
> the patch the reproduction fails because the periodic task keeps running.
> {{checkAllVolumeSets}} still lets any other {{RuntimeException}} end the
> periodic check; catching it there could be a follow up.
> Found by code review of the datanode volume and per-disk RocksDB lifecycle
> paths, as part of the TLA+ verification effort under HDDS-15926, on commit
> cce1ca28944886218132f01c0e271c1fd0700078. 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
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
