yandrey321 opened a new pull request, #11364: URL: https://github.com/apache/ozone/pull/11364
## What changes were proposed in this pull request? `TestOFS.testGetFileStatusUsesSingleOmRpc` fails intermittently in CI with `expected: <500> but was: <501>`. The test asserts an exact delta on two cluster-global `OMMetrics` counters, and an OM background thread advances one of them. `AbstractRootedOzoneFileSystemTest` enables trash with a 3-second emptier interval, so `OzoneManager.startTrashEmptier` runs a `TrashEmptier` daemon over `TrashOzoneFileSystem`. Every emptier tick calls `fs.getTrashRoots(true)`, which iterates *every bucket in the cluster* and calls `exists(trashRoot)` on each. `TrashOzoneFileSystem.exists` delegates to its own `getFileStatus`, which was incrementing the client-facing `numGetFileStatus`. So that counter grew by roughly the cluster's bucket count every 3 seconds, unconditionally — no trash had to exist for it to happen. The test's assertion window is only a few tens of milliseconds wide, which is why the failure is intermittent rather than constant, and why it shows up in the rooted-OFS suite (many buckets) rather than in the older o3fs suite (one or two). Widening the assertion is not a fix: the background noise *adds*, so both "exactly +1" and "exactly +0" break, while "at least +1" would no longer assert the single-RPC property the test exists for. This PR makes two changes. **1. `TrashOzoneFileSystem.getFileStatus` increments the trash counter.** It now calls `incNumTrashGetFileStatus()` instead of `incNumGetFileStatus()`. `OMMetrics` already declared and exported `numTrashGetFileStatus`, but no code ever called its incrementer — this was a dead-counter wiring bug rather than a change in metric semantics. Every other `TrashOzoneFileSystem` override already routes to its own trash counter (`incNumTrashRenames`, `incNumTrashDeletes`, `incNumTrashListStatus`, `incNumTrashExists`, ...); `getFileStatus` was the only one that did not. After the change, `numGetFileStatus` counts client GetFileStatus RPCs only — which is what the test meant to assert — and the `om_metrics` trash counters correctly attribute the emptier's own work. A getter, `getNumTrashGetFileStatus()`, is added alongside, matching the existing `incNumTrashRenames`/`getNumTrashRenames` pairing. **2. The "no InfoBucket RPC" property moves to a unit test.** That property is the HDDS-15925 behaviour the integration test was really guarding: after HDDS-15925 the OM validates the bucket layout server-side, so `BasicRootedOzoneClientAdapterImpl.getFileStatus` calls `getOzoneFileStatus` directly and must not fetch the bucket. Asserting it through the cluster-global `numBucketInfos` counter is indirect and exposed to the same background noise (`TrashOzoneFileSystem.rename`/`delete` also advance `numBucketInfos`). It is now asserted directly on the adapter in `TestBasicRootedOzoneClientAdapterHeadOp#keyPathDoesNotFetchBucket`, with `verify(adapter, never()).getBucket(...)` plus a single `getOzoneFileStatus` verification. The `numBucketInfos` assertions are removed from `testGetFileStatusUsesSingleOmRpc` and `testGetFileStatusRejectsObsBucket`; the rest of both tests, including the OBS rejection message assertions, is unchanged. `testGetFileStatusUsesSingleOmRpc` keeps its `numGetFileStatus` assertions. After change 1 that counter's only remaining producers are the OM read/RPC path (`OmMetadataReader`, `OzoneManagerRequestHandler`, `OzoneManager#rcReader`) and `KeyLifecycleService`, which is disabled by default and runs on a 24-hour interval, so nothing in this suite advances it in the background. `numBucketInfos` itself is deliberately left alone. Its only call site is `OzoneManager.getBucketInfo(volume, bucket)`, which also performs the ACL check, writes the READ_BUCKET audit entry and calls `enrichLinkBucketInfo`. `TrashOzoneFileSystem.rename`/`delete` call it only to read the bucket layout, and bypassing it to avoid the counter bump would skip link-bucket resolution and could report a wrong layout. That belongs in its own Jira rather than in a flaky-test fix. Generated-by: Claude Code (Opus 5) ## What is the link to the Apache JIRA https://issues.apache.org/jira/browse/HDDS-16513 ## How was this patch tested? New unit test, plus the integration tests that the change touches: * `TestBasicRootedOzoneClientAdapterHeadOp` — new `keyPathDoesNotFetchBucket`; suite green, 10/10. * `testTrash` additionally asserts that `numTrashGetFileStatus` advanced, giving the newly-wired counter coverage. * `TestOFS` and `TestOFSWithFSO`, methods `testTrash`, `testGetFileStatusUsesSingleOmRpc` and `testGetFileStatusRejectsObsBucket` — green in both suites, covering both the FSO and non-FSO branch of `testTrash`. -- 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]
