This is an automated email from the ASF dual-hosted git repository.
shashikant pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hadoop-ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 26d8375 HDDS-3703. Avoid file lookup calls in writeChunk hotpath
(#1013)
26d8375 is described below
commit 26d83753792908e840fe6fc9629bed45bb6fa114
Author: Doroszlai, Attila <[email protected]>
AuthorDate: Fri Jun 5 10:35:59 2020 +0200
HDDS-3703. Avoid file lookup calls in writeChunk hotpath (#1013)
---
.../container/common/impl/ChunkLayOutVersion.java | 22 ++++++++--------------
.../ozone/container/keyvalue/KeyValueHandler.java | 7 ++-----
.../container/keyvalue/helpers/ChunkUtils.java | 9 +++++++++
.../keyvalue/impl/ChunkManagerDispatcher.java | 6 +++---
.../keyvalue/impl/FilePerBlockStrategy.java | 12 +++++++-----
.../keyvalue/interfaces/ChunkManager.java | 4 ++--
.../client/rpc/TestContainerStateMachine.java | 6 +++---
7 files changed, 34 insertions(+), 32 deletions(-)
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ChunkLayOutVersion.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ChunkLayOutVersion.java
index ff4ba41..e0341fa 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ChunkLayOutVersion.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ChunkLayOutVersion.java
@@ -41,14 +41,14 @@ public enum ChunkLayOutVersion {
FILE_PER_CHUNK(1, "One file per chunk") {
@Override
public File getChunkFile(File chunkDir, BlockID blockID,
- ChunkInfo info) throws StorageContainerException {
- return chunkDir.toPath().resolve(info.getChunkName()).toFile();
+ ChunkInfo info) {
+ return new File(chunkDir, info.getChunkName());
}
},
FILE_PER_BLOCK(2, "One file per block") {
@Override
public File getChunkFile(File chunkDir, BlockID blockID,
- ChunkInfo info) throws StorageContainerException {
+ ChunkInfo info) {
return new File(chunkDir, blockID.getLocalID() + ".block");
}
};
@@ -117,12 +117,12 @@ public enum ChunkLayOutVersion {
}
public abstract File getChunkFile(File chunkDir,
- BlockID blockID, ChunkInfo info) throws StorageContainerException;
+ BlockID blockID, ChunkInfo info);
public File getChunkFile(ContainerData containerData, BlockID blockID,
ChunkInfo info) throws StorageContainerException {
- File chunksLoc = verifyChunkDirExists(containerData);
- return getChunkFile(chunksLoc, blockID, info);
+ File chunkDir = getChunkDir(containerData);
+ return getChunkFile(chunkDir, blockID, info);
}
@Override
@@ -130,7 +130,7 @@ public enum ChunkLayOutVersion {
return "ChunkLayout:v" + version;
}
- private static File verifyChunkDirExists(ContainerData containerData)
+ private static File getChunkDir(ContainerData containerData)
throws StorageContainerException {
Preconditions.checkNotNull(containerData, "Container data can't be null");
@@ -140,13 +140,7 @@ public enum ChunkLayOutVersion {
throw new StorageContainerException("Unable to get Chunks directory.",
UNABLE_TO_FIND_DATA_DIR);
}
- File chunksLoc = new File(chunksPath);
- if (!chunksLoc.exists()) {
- LOG.error("Chunks path does not exist");
- throw new StorageContainerException("Unable to get Chunks directory.",
- UNABLE_TO_FIND_DATA_DIR);
- }
- return chunksLoc;
+ return new File(chunksPath);
}
}
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
index 26e98c2..1cfd648 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
@@ -421,10 +421,7 @@ public class KeyValueHandler extends Handler {
Preconditions.checkNotNull(blockData);
if (!request.getPutBlock().hasEof() || request.getPutBlock().getEof()) {
- for (ContainerProtos.ChunkInfo chunkInfo : blockData.getChunks()) {
- chunkManager.finishWriteChunk(kvContainer, blockData.getBlockID(),
- ChunkInfo.getFromProtoBuf(chunkInfo));
- }
+ chunkManager.finishWriteChunks(kvContainer, blockData);
}
long bcsId =
@@ -773,7 +770,7 @@ public class KeyValueHandler extends Handler {
// here. There is no need to maintain this info in openContainerBlockMap.
chunkManager
.writeChunk(kvContainer, blockID, chunkInfo, data,
dispatcherContext);
- chunkManager.finishWriteChunk(kvContainer, blockID, chunkInfo);
+ chunkManager.finishWriteChunks(kvContainer, blockData);
List<ContainerProtos.ChunkInfo> chunks = new LinkedList<>();
chunks.add(chunkInfoProto);
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/ChunkUtils.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/ChunkUtils.java
index df0279f..e1fac6f 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/ChunkUtils.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/ChunkUtils.java
@@ -261,6 +261,15 @@ public final class ChunkUtils {
return Boolean.parseBoolean(overWrite);
}
+ public static void verifyChunkFileExists(File file)
+ throws StorageContainerException {
+ if (!file.exists()) {
+ throw new StorageContainerException(
+ "Chunk file not found: " + file.getPath(),
+ UNABLE_TO_FIND_CHUNK);
+ }
+ }
+
@VisibleForTesting
static <T> T processFileExclusively(Path path, Supplier<T> op) {
for (;;) {
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/ChunkManagerDispatcher.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/ChunkManagerDispatcher.java
index 42e96a1..3d2b815 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/ChunkManagerDispatcher.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/ChunkManagerDispatcher.java
@@ -70,11 +70,11 @@ public class ChunkManagerDispatcher implements ChunkManager
{
}
@Override
- public void finishWriteChunk(KeyValueContainer kvContainer, BlockID blockID,
- ChunkInfo info) throws IOException {
+ public void finishWriteChunks(KeyValueContainer kvContainer,
+ BlockData blockData) throws IOException {
selectHandler(kvContainer)
- .finishWriteChunk(kvContainer, blockID, info);
+ .finishWriteChunks(kvContainer, blockData);
}
@Override
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/FilePerBlockStrategy.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/FilePerBlockStrategy.java
index 2aa89a4..7e7e5f4 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/FilePerBlockStrategy.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/FilePerBlockStrategy.java
@@ -55,6 +55,7 @@ import static
org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Res
import static
org.apache.hadoop.ozone.container.common.impl.ChunkLayOutVersion.FILE_PER_BLOCK;
import static
org.apache.hadoop.ozone.container.common.transport.server.ratis.DispatcherContext.WriteChunkStage.COMMIT_DATA;
import static
org.apache.hadoop.ozone.container.keyvalue.helpers.ChunkUtils.validateChunkForOverwrite;
+import static
org.apache.hadoop.ozone.container.keyvalue.helpers.ChunkUtils.verifyChunkFileExists;
/**
* This class is for performing chunk related operations.
@@ -163,10 +164,11 @@ public class FilePerBlockStrategy implements ChunkManager
{
}
@Override
- public void finishWriteChunk(KeyValueContainer container, BlockID blockID,
- ChunkInfo info) throws IOException {
- File chunkFile = getChunkFile(container, blockID, info);
+ public void finishWriteChunks(KeyValueContainer container,
+ BlockData blockData) throws IOException {
+ File chunkFile = getChunkFile(container, blockData.getBlockID(), null);
files.close(chunkFile);
+ verifyChunkFileExists(chunkFile);
}
private void deleteChunk(Container container, BlockID blockID,
@@ -227,7 +229,7 @@ public class FilePerBlockStrategy implements ChunkManager {
public FileChannel getChannel(File file, boolean sync)
throws StorageContainerException {
try {
- return files.get(file.getAbsolutePath(),
+ return files.get(file.getPath(),
() -> open(file, sync)).getChannel();
} catch (ExecutionException e) {
if (e.getCause() instanceof IOException) {
@@ -248,7 +250,7 @@ public class FilePerBlockStrategy implements ChunkManager {
public void close(File file) {
if (file != null) {
- files.invalidate(file.getAbsolutePath());
+ files.invalidate(file.getPath());
}
}
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/interfaces/ChunkManager.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/interfaces/ChunkManager.java
index fe49e84..8db72f7 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/interfaces/ChunkManager.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/interfaces/ChunkManager.java
@@ -97,8 +97,8 @@ public interface ChunkManager {
// if applicable
}
- default void finishWriteChunk(KeyValueContainer kvContainer, BlockID blockID,
- ChunkInfo info) throws IOException {
+ default void finishWriteChunks(KeyValueContainer kvContainer,
+ BlockData blockData) throws IOException {
// no-op
}
}
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestContainerStateMachine.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestContainerStateMachine.java
index 1972dac..6431b70 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestContainerStateMachine.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestContainerStateMachine.java
@@ -156,12 +156,12 @@ public class TestContainerStateMachine {
key.close();
// Make sure the container is marked unhealthy
- Assert.assertTrue(
+ Assert.assertEquals(
+ ContainerProtos.ContainerDataProto.State.UNHEALTHY,
cluster.getHddsDatanodes().get(0).getDatanodeStateMachine()
.getContainer().getContainerSet()
.getContainer(omKeyLocationInfo.getContainerID())
- .getContainerState()
- == ContainerProtos.ContainerDataProto.State.UNHEALTHY);
+ .getContainerState());
}
@Test
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
