umamaheswararao commented on code in PR #4104:
URL: https://github.com/apache/ozone/pull/4104#discussion_r1062074268
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/RatisBlockOutputStream.java:
##########
@@ -126,4 +128,16 @@ void waitOnFlushFutures() throws InterruptedException,
ExecutionException {
void cleanup() {
commitWatcher.cleanup();
}
+
+ @Override
+ public void hflush() throws IOException {
+ hsync();
+ }
+
+ @Override
+ public void hsync() throws IOException {
+ if (!isClosed()) {
Review Comment:
watchForCommit is basically to make sure buffers are properly send and
stored at DNs.
SO, here is the place we need to implement to notify to OM that, here is a
hsync call from app, we need to update the block info along with the so far
committed content right? If, yes, do you mind adding a todo comment here to
make it more clear? If we have already filed followup JIRA for om handling, may
be a good idea to put in the comment to make it more clear if anybody wants to
review.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.ozone;
+
+import org.apache.hadoop.conf.StorageUnit;
+import org.apache.hadoop.fs.CommonConfigurationKeysPublic;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.TestDataUtil;
+import org.apache.hadoop.ozone.client.OzoneBucket;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.junit.AfterClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.jupiter.api.Assertions;
+import org.junit.rules.Timeout;
+
+import java.util.concurrent.ThreadLocalRandom;
+
+import static org.apache.hadoop.ozone.OzoneConsts.OZONE_OFS_URI_SCHEME;
+import static org.apache.hadoop.ozone.OzoneConsts.OZONE_ROOT;
+import static org.apache.hadoop.ozone.OzoneConsts.OZONE_URI_DELIMITER;
+import static org.apache.hadoop.ozone.OzoneConsts.OZONE_URI_SCHEME;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT;
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_ADDRESS_KEY;
+import static
org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_RATIS_ENABLE_KEY;
+
+
+/**
+ * Test HSync.
+ */
+public class TestHSync {
+ @Rule
+ public Timeout timeout = Timeout.seconds(300);
+
+ private static MiniOzoneCluster cluster;
+ private static OzoneBucket bucket;
+
+ private final OzoneConfiguration conf = new OzoneConfiguration();
+
+ {
+ try {
+ init();
+ } catch (Exception e) {
+ throw new IllegalStateException(e);
+ }
+ }
+
+ private void init() throws Exception {
+ final int chunkSize = 16 << 10;
+ final int flushSize = 2 * chunkSize;
+ final int maxFlushSize = 2 * flushSize;
+ final int blockSize = 2 * maxFlushSize;
+ final BucketLayout layout = BucketLayout.FILE_SYSTEM_OPTIMIZED;
+
+ conf.setBoolean(OZONE_OM_RATIS_ENABLE_KEY, false);
+ conf.set(OZONE_DEFAULT_BUCKET_LAYOUT, layout.name());
+ cluster = MiniOzoneCluster.newBuilder(conf)
+ .setNumDatanodes(5)
+ .setTotalPipelineNumLimit(10)
+ .setBlockSize(blockSize)
+ .setChunkSize(chunkSize)
+ .setStreamBufferFlushSize(flushSize)
+ .setStreamBufferMaxSize(maxFlushSize)
+ .setDataStreamBufferFlushize(maxFlushSize)
+ .setStreamBufferSizeUnit(StorageUnit.BYTES)
+ .setDataStreamMinPacketSize(chunkSize)
+ .setDataStreamStreamWindowSize(5 * chunkSize)
+ .build();
+ cluster.waitForClusterToBeReady();
+
+ // create a volume and a bucket to be used by OzoneFileSystem
+ bucket = TestDataUtil.createVolumeAndBucket(cluster, layout);
+ }
+
+ @AfterClass
+ public static void teardown() {
+ if (cluster != null) {
+ cluster.shutdown();
+ }
+ }
+
+ @Test
+ public void testO3fsHSync() throws Exception {
+ // Set the fs.defaultFS
+ final String rootPath = String.format("%s://%s.%s/",
+ OZONE_URI_SCHEME, bucket.getName(), bucket.getVolumeName());
+ conf.set(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY, rootPath);
+
+ final Path file = new Path("/file");
+
+ try (FileSystem fs = FileSystem.get(conf)) {
+ runTestHSync(fs, file);
+ }
+ }
+
+ @Test
+ public void testOfsHSync() throws Exception {
+ // Set the fs.defaultFS
+ final String rootPath = String.format("%s://%s/",
+ OZONE_OFS_URI_SCHEME, conf.get(OZONE_OM_ADDRESS_KEY));
+ conf.set(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY, rootPath);
+
+ final String dir = OZONE_ROOT + bucket.getVolumeName()
+ + OZONE_URI_DELIMITER + bucket.getName();
+ final Path file = new Path(dir, "file");
+
+ try (FileSystem fs = FileSystem.get(conf)) {
+ runTestHSync(fs, file);
+ }
+ }
+
+ static void runTestHSync(FileSystem fs, Path file) throws Exception {
+ final byte[] data = new byte[1 << 20];
+ ThreadLocalRandom.current().nextBytes(data);
+
+ final FSDataOutputStream stream = fs.create(file, true);
+ stream.write(data);
+ stream.hsync();
Review Comment:
I guess you would cover multiple hsync test once om changes done for better
assertion of contents once read.
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/BlockOutputStreamEntry.java:
##########
@@ -47,7 +48,7 @@
public class BlockOutputStreamEntry extends OutputStream {
private final OzoneClientConfig config;
- private OutputStream outputStream;
+ private RatisBlockOutputStream outputStream;
Review Comment:
Currently BlockOutputtreamEntry extended by ECBlockOutputStream entry. It
may be a good idea to keep the BlockOutputStreamEntry as generic than making
more specific to Ratis?
--
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]
