jojochuang commented on PR #11118: URL: https://github.com/apache/ozone/pull/11118#issuecomment-5428326910
## Pre-PR vs post-PR: bucket lock behavior during FSO directory purge Two distinct lock-hold problems are easy to conflate because both show up as long OM apply times, but they have different causes and different fixes. ### 1. Large single-bucket batches (still a hotspot, but cheaper) Before this PR, a single `PurgeDirectories` transaction for one bucket could contain many sub-dirs/sub-files (e.g. one deleted directory tree). On apply, `OMDirectoriesPurgeRequestWithFSO` held that bucket's write lock for the **entire** loop, including: - full `OmKeyInfo.getFromProtobuf()` per entry - `sumBlockLengths()` on parsed keys - hsync / open-key lookup - per-entry `getBucketInfo()` and quota decrements - cache tombstones So large single-bucket batches already caused long write-lock holds on that bucket. This PR does **not** split those batches further (HDDS-16297), but it **does** move parse/precompute outside the lock and keeps only tombstones + batched quota work under the lock. Same batch size, shorter hold. ### 2. Multiple buckets in one transaction (fixed by this PR, AOS path) Terminology matters here: - One **`PurgePathRequest`** is always one bucket (one `volumeId`/`bucketId`). - One **`PurgeDirectoriesRequest`** / `OMDirectoriesPurgeRequestWithFSO` apply can contain **many** `PurgePathRequest`s. Before this PR, `DirectoryDeletingService.submitPurgePathsWithBatching` packed paths purely by Ratis byte limit, with **no** bucket boundary. On the **AOS** path (`snapshotId == null`), DDS walks the global deleted-dir table and can accumulate paths from different buckets in one worker iteration; the code even notes AOS "could process multiple buckets in one iteration." Those mixed-bucket paths could land in one submitted transaction. On apply, `getBucketLockKeySet()` collects every distinct bucket in the request and `acquireWriteLocks(BUCKET_LOCK, bucketLockKeys)` held **all** of those write locks for the whole apply loop. A background purge could therefore block read RPCs on unrelated buckets while backlog was drained elsewhere. **Snapshot deep-clean** was already single-bucket end-to-end: the deleted-dir supplier is scoped to one `(volume, bucket)` per snapshot task. This PR groups DDS submissions by `(volumeId, bucketId)` before byte-limit batching, so each submitted AOS transaction touches **one bucket** and apply takes **one** write lock. ### Summary | Scenario | Before | After | |----------|--------|-------| | Large batch, one bucket | Long write-lock hold (parse + apply under lock) | Still one lock per txn, but shorter hold (apply-only under lock) | | One txn spanning multiple buckets (AOS) | Multiple write locks held together | Fixed — one bucket per txn | | Snapshot deep-clean | Single bucket (unchanged) | Single bucket (unchanged) | | Very large single `PurgePathRequest` | Long single-bucket hold | Still possible; not split yet (HDDS-16297) | The remaining hotspot called out in review is the first and last rows: dense single-bucket work is cheaper now, but a very large purge path can still hold one bucket write lock for the full apply phase until HDDS-16297 lands. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
