pan3793 commented on PR #58312: URL: https://github.com/apache/spark/pull/58312#issuecomment-5506812009
Thanks for the thorough review, and for the scratch test. 1. Restructured: `openStore()` now runs on its own thread, and the parked `release()` is let go once that thread has finished or blocked on the lock, so there is no fixed wait. The test now asserts `openStore()` returns `None`; the dead block is gone. Both suites finish in ~20 ms. 2. Closed it here. `commit()` now renames the store, updates usage, writes the listing entry, and registers the app in `active` under one lock. Added `SPARK-58985: commit is atomic with release`, which fails on the previous revision with the same `Disk usage tracker went negative` exception. Since the listing entry and the `active` entry are written under the same lock, `makeRoom()` can no longer collect a store that `commit()` is about to register. 3. Done: two counters in `makeRoom()`; `openStore()` writes the listing entry under the lock; the description's trade-off paragraph now covers `makeRoom()` too. Also fixed the log keys on the eviction summary line, where the store count was labeled `NUM_BYTES_EVICTED`. On the stale `info.size` you noted: `makeRoom()` now re-reads the listing entry under the lock and deducts that size. A candidate whose entry is gone was deleted concurrently and is skipped; one whose directory is gone but entry remains (deleted out of band) is still deducted as on master, which the previous revision had lost. `release()` drops the listing entry when the directory is gone so that store is not deducted again later, and otherwise keeps the base body, only wrapped in the lock, to keep the diff small for the backports. Added `makeRoom is atomic with release` and `makeRoom deducts a store deleted out of band` for these. -- 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]
