chihsuan commented on code in PR #11151:
URL: https://github.com/apache/ozone/pull/11151#discussion_r3905358936
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerData.java:
##########
@@ -263,6 +269,26 @@ public String getChunksPath() {
*/
public void setChunksPath(String chunkPath) {
this.chunksPath = chunkPath;
+ this.chunksDirFile = null;
+ }
+
+ /**
+ * Read-path accessor for the chunks directory. Resolves and validates it
once
+ * via {@link ContainerUtils#getChunkDir(ContainerData)}, then returns the
+ * cached result on later calls to skip the per-read stat. Writes and other
+ * callers use {@code ContainerUtils.getChunkDir} directly, so a missing
+ * directory still surfaces as a storage failure on every operation.
+ *
+ * @return the resolved chunks directory
+ * @throws StorageContainerException if the chunks directory cannot be
resolved
+ */
+ public File getChunksDirForRead() throws StorageContainerException {
Review Comment:
Should this getter be `@JsonIgnore`?
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/FilePerChunkStrategy.java:
##########
@@ -224,8 +224,10 @@ public ChunkBufferToByteString readChunk(Container
container, BlockID blockID,
HddsVolume volume = containerData.getVolume();
// In version1, we verify checksum if it is available and return data
- // of the chunk file.
- File finalChunkFile = getChunkFile(kvContainer, blockID, info);
+ // of the chunk file. Reads use the cached chunks directory to skip the
+ // per-read stat; writes keep validating so a missing directory is
detected.
+ File finalChunkFile = FILE_PER_CHUNK.getChunkFile(
+ containerData.getChunksDirForRead(), blockID, info.getChunkName());
Review Comment:
Should we still validate the directory at this path to preserve the previous
error and logging behavior?
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/helpers/ContainerUtils.java:
##########
@@ -298,7 +298,10 @@ public static File getContainerFile(File containerBaseDir)
{
}
/**
- * Get the chunk directory from the containerData.
+ * Resolve and validate the chunk directory from the containerData. The
+ * directory's existence is checked on every call, so a chunk operation
+ * against a failed volume surfaces as a storage failure; write paths rely on
+ * this to mark the container unhealthy.
Review Comment:
nit: Could this Javadoc stay about what the method does? It now describes
what callers rely on, and the same note appears in other files.
--
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]
