Skip to content

Conversation

@guohao-rosicky
Copy link
Contributor

What changes were proposed in this pull request?

KeyValueStreamDataChannel add write data exception handling

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-8220

@kerneltime kerneltime requested a review from szetszwo March 20, 2023 16:08
@kerneltime
Copy link
Contributor

@hemantk-12 can you please take a look?

Copy link
Contributor

@hemantk-12 hemantk-12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good to me.

Left some minor comments.

return request;
}

private void volumeOnFailure() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I don't think it is ideal to create function for one liner.

super.close();
try {
putBlockRequest.set(closeBuffers(buffers, super::writeFileChannel));
super.close();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should super.close() be out of the try-catch?

Comment on lines 290 to 291
private void volumeOnFailure() {
onFailure(getContainerData().getVolume());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@guohao-rosicky , thanks for working on this!

Let's move this method up and rename it to checkVolume.

+++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/StreamDataChannelBase.java
@@ -22,6 +22,7 @@
 import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException;
 import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics;
 import org.apache.hadoop.ozone.container.common.impl.ContainerData;
+import org.apache.hadoop.ozone.container.common.utils.StorageVolumeUtil;
 import org.apache.ratis.statemachine.StateMachine;
 
 import java.io.File;
@@ -64,6 +65,10 @@ private FileChannel getChannel() {
     return randomAccessFile.getChannel();
   }
 
+  void checkVolume() {
+    StorageVolumeUtil.onFailure(containerData.getVolume());
+  }
+
   @Override
   public final void force(boolean metadata) throws IOException {
     getChannel().force(metadata);

Copy link
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 the change looks good.

@adoroszlai adoroszlai changed the title HDDS-8220. [Ozone-Streaming] KeyValueStreamDataChannel add write data exception handling HDDS-8220. [Ozone-Streaming] Trigger volume check on IOException in StreamDataChannelBase Mar 22, 2023
@adoroszlai adoroszlai merged commit 8e0c462 into apache:master Mar 22, 2023
@adoroszlai
Copy link
Contributor

Thanks @guohao-rosicky for the patch, @hemantk-12, @szetszwo for the review.

errose28 added a commit to errose28/ozone that referenced this pull request Mar 23, 2023
* master: (43 commits)
  HDDS-8148. Improve log for Pipeline creation failure (apache#4385)
  HDDS-7853. Add support for RemoveSCM in SCMRatisServer. (apache#4358)
  HDDS-8042. Display certificate issuer in cert list command. (apache#4429)
  HDDS-8189. [Snapshot] renamedKeyTable should only track keys in buckets that has at least one active snapshot. (apache#4436)
  HDDS-8154. Perf: Reuse Mac instances in S3 token validation (apache#4433)
  HDDS-8245. Info log for keyDeletingService when nonzero number of keys are deleted. (apache#4451)
  HDDS-8233. ReplicationManager: Throttle delete container commands from over-replication handlers (apache#4447)
  HDDS-8220. [Ozone-Streaming] Trigger volume check on IOException in StreamDataChannelBase (apache#4428)
  HDDS-8173. Fix to remove enrties from RocksDB after container gets deleted. (apache#4445)
  HDDS-7975. Rebalance acceptance tests (apache#4437)
  HDDS-8152. Reduce S3 acceptance test setup time (apache#4393)
  HDDS-8172. ECUnderReplicationHandler should consider commands already sent when processing the container (apache#4435)
  HDDS-7883. [Snapshot] Accommodate FSO, key renames and implement OMSnapshotPurgeRequest for SnapshotDeletingService (apache#4407)
  HDDS-8168. Make deadlines inside MoveManager for move commands configurable (apache#4415)
  HDDS-7918. EC: ECBlockReconstructedStripeInputStream should check for spare replicas before failing an index (apache#4441)
  HDDS-8222. EndpointBase#getBucket should handle BUCKET_NOT_FOUND (apache#4431)
  HDDS-8068. Fix Exception: JMXJsonServlet, getting attribute RatisRoles of Hadoop:service=OzoneManager. (apache#4352)
  HDDS-8139. Datanodes should not drop block delete transactions based on transaction ID (apache#4384)
  HDDS-8216. EC: OzoneClientConfig is overwritten in ECKeyOutputStream (apache#4425)
  HDDS-8054. Fix NPE in metrics for failed volume (apache#4340)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants