chihsuan commented on code in PR #11752:
URL: https://github.com/apache/ozone/pull/11752#discussion_r4374710241


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/ClientVersion.java:
##########
@@ -42,6 +42,9 @@ public enum ClientVersion implements ComponentVersion {
       "This client version has support for Object Store and File " +
           "System Optimized Bucket Layouts."),
 
+  KEY_ALLOCATION_PIPELINE_NOT_FOUND(4,

Review Comment:
   This bump breaks released clients. `OLDER_CLIENT_REQUESTS` compares against 
`CURRENT`, so v3 clients are now treated as old, and `CreateKey` on OBS/FSO 
buckets fails with `NOT_SUPPORTED_OPERATION`. Could we reuse the unused 
`KEY_ALLOCATION_ERROR` instead and drop the bump?



##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java:
##########
@@ -2627,33 +2642,36 @@ public OpenKeySession createFile(OmKeyArgs args,
     OMRequest omRequest = createOMRequest(Type.CreateFile)
         .setCreateFileRequest(createFileRequest)
         .build();
-    CreateFileResponse resp = 
handleSubmitRequestAndSCMSafeModeRetry(omRequest).getCreateFileResponse();
+    CreateFileResponse resp = 
handleSubmitRequestWithBlockAllocationRetry(omRequest).getCreateFileResponse();
 
     return new OpenKeySession(resp.getID(),
         OmKeyInfo.getFromProtobuf(resp.getKeyInfo()), resp.getOpenVersion());
   }
 
   @Nonnull
-  private OMResponse handleSubmitRequestAndSCMSafeModeRetry(OMRequest 
omRequest) throws IOException {
-    int retryCount = BLOCK_ALLOCATION_RETRY_COUNT;
+  private OMResponse handleSubmitRequestWithBlockAllocationRetry(OMRequest 
omRequest) throws IOException {
+    int retryCount = blockAllocationRetryCount;
     while (true) {
       try {
         return handleError(submitRequest(omRequest));
       } catch (OMException e) {
-        if (e.getResult().equals(SCM_IN_SAFE_MODE) && retryCount > 0) {
-          System.err.println("SCM is in safe mode. Will retry in " +
-              BLOCK_ALLOCATION_RETRY_WAIT_TIME_MS + "ms");
-          retryCount--;
-          try {
-            Thread.sleep(BLOCK_ALLOCATION_RETRY_WAIT_TIME_MS);
-            continue;
-          } catch (InterruptedException ex) {
-            throw new OMException(ex.getMessage(), 
ResultCodes.SCM_IN_SAFE_MODE);
-          }
-        } else if (e.getResult().equals(SCM_IN_SAFE_MODE) && retryCount == 0) {
-          throw new OMException(e.getMessage(), ResultCodes.SCM_IN_SAFE_MODE);
+        if ((e.getResult() != SCM_IN_SAFE_MODE && e.getResult() != 
KEY_ALLOCATION_PIPELINE_NOT_FOUND)
+            || retryCount == 0) {
+          throw e;
+        }
+        if (retryCount == blockAllocationRetryCount) {
+          LOG.warn("Block allocation failed with {}. Retrying up to {} times 
at {}ms intervals",

Review Comment:
   This change makes `ozone sh` silent for up to 90 seconds while retrying, 
even in safe mode, since CLI logging is off by default. Could we keep a short 
stderr message here, like 
[`SCMFailoverProxyProviderBase`](https://github.com/apache/ozone/blob/1f9f4cc7918e9c2ef1364b84e04f90289b883c8e/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMFailoverProxyProviderBase.java#L665)
 does for its retries?



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/WritableRatisContainerProvider.java:
##########
@@ -136,9 +143,12 @@ public ContainerInfo getContainer(final long size,
     LOG.error(
         "Unable to allocate a block for the size: {}, repConfig: {}",
         size, repConfig);
-    throw new IOException(
-        "Unable to allocate a container to the block of size: " + size
-            + ", replicationConfig: " + repConfig + ". " + failureReason);
+    String message = "Unable to allocate a container to the block of size: " + 
size
+        + ", replicationConfig: " + repConfig + ". " + failureReason;
+    if (missingPipeline != null) {

Review Comment:
   If the waited pipeline opens and then closes right away, the client gets 
`INTERNAL_ERROR` without retrying. Could we make this retryable too, or handle 
it in a follow-up?



-- 
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]

Reply via email to