cchung100m commented on PR #11435: URL: https://github.com/apache/ozone/pull/11435#issuecomment-6075677501
Hi @peterxcli Thanks for the suggestions. I looked into this and could not find a local refactor that removes the checks without changing a contract. The two `null`s come from existing behavior: 1. `newKeyInfo == null`: the retry function in `RpcClient#getInputStreamWithRetryFunction` catches `IOException` and returns `null`, and `BlockExtendedInputStream#refreshBlockInfo` already treats a `null` result as "no new location, warn and keep retrying". 2. `getLatestVersionLocations() == null`: `OmKeyInfo` returns `null` when `keyLocationVersions` is empty, and several callers check for it (for example, `ChunkKeyHandler`, `OMMultipartUploadUtils`, `SnapshotUtils`, `KeyManagerImpl#slimLocationVersion`). Returning an empty group instead of `null` would change the contract for many callers. In particular, `OmKeyInfo#updateLocationInfoList` calls `setMultipartKey(...)` on the returned group, and several callers read `getVersion()`, so an empty placeholder could silently hide an update that today fails loudly. Making the retry function throw instead of returning `null` would change the read behavior from warn-and-retry to failing the read. So I kept the guard in `getBlockLocationInfo`, which is where the existing "nullable refresh result" contract applies. If you think a non-null `getLatestVersionLocations()` is worth pursuing, I'm happy to file a separate Jira for it. Please let me know if I missed a simpler approach. -- 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]
