liurenjie1024 commented on PR #17236: URL: https://github.com/apache/iceberg/pull/17236#issuecomment-5408400190
I found three issues in the current head: 1. **Blocker: the new Aliyun and Dell tests do not compile.** In both `TestOSSInputStream.CachingMetricsContext` and `TestEcsSeekableInputStream.CachingMetricsContext`, the simple name `Counter` resolves to the inherited deprecated `MetricsContext.Counter`, so the `counter(String, Unit)` override has an incompatible return type. Both the map value type and method return type should use `org.apache.iceberg.metrics.Counter`, matching the other new test helpers. This explains the two red CI jobs; after fixing Aliyun locally, Dell exposes the same compilation failure. 2. **The analytics-core GCS `readTail` path still counts an operation at EOF.** In `AnalyticsCoreUtil.GcsInputStreamWrapper.readTail`, `readOperations.increment()` is outside the `bytesRead > 0` guard. A `-1` or zero result therefore counts a no-data operation, contrary to the convention applied elsewhere in this PR. Moving both increments into the guard and adding a wrapper-tail regression test would make this consistent. I confirmed locally that a mocked `readTail` returning `-1` increments operations from 0 to 1. 3. **Zero-length analytics vectored ranges count phantom operations.** `FileRange` permits `length == 0`, but the new `readVectored` metrics loop increments `READ_OPERATIONS` for every range. Please guard both counters with `range.length() > 0`, consistent with the new `readFully` handling. After applying only the two test-helper type qualifications in a disposable worktree, all seven affected test classes passed. `git diff --check` also passed. -- 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]
