You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/10/05 10:11:35 UTC
[GitHub] [ozone] fapifta opened a new pull request #2709: HDDS-5816 Rearrange code and refactor some logic into new methods in prep for EC addition.
fapifta opened a new pull request #2709:
URL: https://github.com/apache/ozone/pull/2709
## What changes were proposed in this pull request?
During Erasure Coding development, we would like to reuse BlockOutputStreamEntryPool, BlockOutputStreamEntry, and BlockOutputStream, to handle most of the work as the general way of handling writes, and only add specialisations as needed.
In order to prepare for this, some changes are necessary in the basic classes, this JIRA is to bring in those changes to the master so that we do not have too distinct versions in the two branch making merges much more complicated than necessary.
Some more changes might be required during the development, these are related to the changes in HDDS-5755.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5816
Please replace this section with the link to the Apache JIRA)
## How was this patch tested?
junit+CI, changes should no change how the logic works, no new tests added.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] fapifta commented on a change in pull request #2709: HDDS-5816 Rearrange code and refactor some logic into new methods in prep for EC addition.
Posted by GitBox <gi...@apache.org>.
fapifta commented on a change in pull request #2709:
URL: https://github.com/apache/ozone/pull/2709#discussion_r722181814
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/BlockOutputStreamEntry.java
##########
@@ -76,74 +83,94 @@ private BlockOutputStreamEntry(
this.bufferPool = bufferPool;
}
- long getLength() {
- return length;
- }
-
- Token<OzoneBlockTokenIdentifier> getToken() {
- return token;
- }
-
- long getRemaining() {
- return length - currentPosition;
- }
-
/**
* BlockOutputStream is initialized in this function. This makes sure that
* xceiverClient initialization is not done during preallocation and only
* done when data is written.
* @throws IOException if xceiverClient initialization fails
*/
- private void checkStream() throws IOException {
- if (this.outputStream == null) {
- this.outputStream =
- new RatisBlockOutputStream(blockID, xceiverClientManager,
- pipeline, bufferPool, config, token);
+ void checkStream() throws IOException {
+ if (!isInitialized()) {
+ createOutputStream();
}
}
+ /**
+ * Creates the outputStreams that are necessary to start the write.
+ * Implementors can override this to instantiate multiple streams instead.
+ * @throws IOException
+ */
+ void createOutputStream() throws IOException {
+ outputStream = new RatisBlockOutputStream(blockID, xceiverClientManager,
+ pipeline, bufferPool, config, token);
+ }
@Override
public void write(int b) throws IOException {
checkStream();
- outputStream.write(b);
- this.currentPosition += 1;
+ getOutputStream().write(b);
+ incCurrentPosition();
}
@Override
public void write(byte[] b, int off, int len) throws IOException {
checkStream();
- outputStream.write(b, off, len);
- this.currentPosition += len;
+ getOutputStream().write(b, off, len);
+ incCurrentPosition(len);
+ }
+
+ void writeOnRetry(long len) throws IOException {
+ checkStream();
+ BlockOutputStream out = (BlockOutputStream) getOutputStream();
+ out.writeOnRetry(len);
+ incCurrentPosition(len);
}
@Override
public void flush() throws IOException {
- if (this.outputStream != null) {
- this.outputStream.flush();
+ if (isInitialized()) {
+ getOutputStream().flush();
}
}
@Override
public void close() throws IOException {
- if (this.outputStream != null) {
- this.outputStream.close();
+ if (isInitialized()) {
+ getOutputStream().close();
// after closing the chunkOutPutStream, blockId would have been
// reconstructed with updated bcsId
- this.blockID = ((BlockOutputStream) outputStream).getBlockID();
+ this.blockID = ((BlockOutputStream) getOutputStream()).getBlockID();
}
}
boolean isClosed() {
- if (outputStream != null) {
- return ((BlockOutputStream) outputStream).isClosed();
+ if (isInitialized()) {
+ return ((BlockOutputStream) getOutputStream()).isClosed();
}
return false;
}
+ void cleanup(boolean invalidateClient) throws IOException {
+ checkStream();
+ BlockOutputStream out = (BlockOutputStream) getOutputStream();
+ out.cleanup(invalidateClient);
+
Review comment:
removed, thank you for spotting it Stephen!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] fapifta merged pull request #2709: HDDS-5816 Rearrange code and refactor some logic into new methods in prep for EC addition.
Posted by GitBox <gi...@apache.org>.
fapifta merged pull request #2709:
URL: https://github.com/apache/ozone/pull/2709
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] fapifta commented on a change in pull request #2709: HDDS-5816 Rearrange code and refactor some logic into new methods in prep for EC addition.
Posted by GitBox <gi...@apache.org>.
fapifta commented on a change in pull request #2709:
URL: https://github.com/apache/ozone/pull/2709#discussion_r722775997
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/BlockOutputStreamEntry.java
##########
@@ -36,9 +36,16 @@
import com.google.common.annotations.VisibleForTesting;
/**
- * Helper class used inside {@link BlockOutputStream}.
+ * A BlockOutputStreamEntry manages the data writes into the DataNodes.
+ * It wraps BlockOutputStreams that are connecting to the DataNodes,
+ * and in the meantime accounts the length of data successfully written.
+ *
+ * The base implementation is handling Ratis-3 writes, with a single stream,
+ * but there can be other implementations that are using a different way, like
+ * EC writes where an entry manages the write into a block group via multiple
Review comment:
Done, thank you for noticing it.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] umamaheswararao commented on a change in pull request #2709: HDDS-5816 Rearrange code and refactor some logic into new methods in prep for EC addition.
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on a change in pull request #2709:
URL: https://github.com/apache/ozone/pull/2709#discussion_r722621445
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/BlockOutputStreamEntry.java
##########
@@ -36,9 +36,16 @@
import com.google.common.annotations.VisibleForTesting;
/**
- * Helper class used inside {@link BlockOutputStream}.
+ * A BlockOutputStreamEntry manages the data writes into the DataNodes.
+ * It wraps BlockOutputStreams that are connecting to the DataNodes,
+ * and in the meantime accounts the length of data successfully written.
+ *
+ * The base implementation is handling Ratis-3 writes, with a single stream,
+ * but there can be other implementations that are using a different way, like
+ * EC writes where an entry manages the write into a block group via multiple
Review comment:
Let's avoid to have EC related docs in master code.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sodonnel commented on a change in pull request #2709: HDDS-5816 Rearrange code and refactor some logic into new methods in prep for EC addition.
Posted by GitBox <gi...@apache.org>.
sodonnel commented on a change in pull request #2709:
URL: https://github.com/apache/ozone/pull/2709#discussion_r722167460
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/BlockOutputStreamEntry.java
##########
@@ -76,74 +83,94 @@ private BlockOutputStreamEntry(
this.bufferPool = bufferPool;
}
- long getLength() {
- return length;
- }
-
- Token<OzoneBlockTokenIdentifier> getToken() {
- return token;
- }
-
- long getRemaining() {
- return length - currentPosition;
- }
-
/**
* BlockOutputStream is initialized in this function. This makes sure that
* xceiverClient initialization is not done during preallocation and only
* done when data is written.
* @throws IOException if xceiverClient initialization fails
*/
- private void checkStream() throws IOException {
- if (this.outputStream == null) {
- this.outputStream =
- new RatisBlockOutputStream(blockID, xceiverClientManager,
- pipeline, bufferPool, config, token);
+ void checkStream() throws IOException {
+ if (!isInitialized()) {
+ createOutputStream();
}
}
+ /**
+ * Creates the outputStreams that are necessary to start the write.
+ * Implementors can override this to instantiate multiple streams instead.
+ * @throws IOException
+ */
+ void createOutputStream() throws IOException {
+ outputStream = new RatisBlockOutputStream(blockID, xceiverClientManager,
+ pipeline, bufferPool, config, token);
+ }
@Override
public void write(int b) throws IOException {
checkStream();
- outputStream.write(b);
- this.currentPosition += 1;
+ getOutputStream().write(b);
+ incCurrentPosition();
}
@Override
public void write(byte[] b, int off, int len) throws IOException {
checkStream();
- outputStream.write(b, off, len);
- this.currentPosition += len;
+ getOutputStream().write(b, off, len);
+ incCurrentPosition(len);
+ }
+
+ void writeOnRetry(long len) throws IOException {
+ checkStream();
+ BlockOutputStream out = (BlockOutputStream) getOutputStream();
+ out.writeOnRetry(len);
+ incCurrentPosition(len);
}
@Override
public void flush() throws IOException {
- if (this.outputStream != null) {
- this.outputStream.flush();
+ if (isInitialized()) {
+ getOutputStream().flush();
}
}
@Override
public void close() throws IOException {
- if (this.outputStream != null) {
- this.outputStream.close();
+ if (isInitialized()) {
+ getOutputStream().close();
// after closing the chunkOutPutStream, blockId would have been
// reconstructed with updated bcsId
- this.blockID = ((BlockOutputStream) outputStream).getBlockID();
+ this.blockID = ((BlockOutputStream) getOutputStream()).getBlockID();
}
}
boolean isClosed() {
- if (outputStream != null) {
- return ((BlockOutputStream) outputStream).isClosed();
+ if (isInitialized()) {
+ return ((BlockOutputStream) getOutputStream()).isClosed();
}
return false;
}
+ void cleanup(boolean invalidateClient) throws IOException {
+ checkStream();
+ BlockOutputStream out = (BlockOutputStream) getOutputStream();
+ out.cleanup(invalidateClient);
+
Review comment:
nit: extra line here
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org