You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "jojochuang (via GitHub)" <gi...@apache.org> on 2023/02/01 22:53:38 UTC

[GitHub] [ozone] jojochuang opened a new pull request, #4234: HDDS-7769. Implement client initiated lease recovery

jojochuang opened a new pull request, #4234:
URL: https://github.com/apache/ozone/pull/4234

   ## What changes were proposed in this pull request?
   
   Implement recoverLease() API that is similar to HDFS's one, which allows a client to forcefully close an open file. 
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-7769
   
   ## How was this patch tested?
   
   Unit tests


-- 
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] szetszwo commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1148485925


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,278 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private String normalizedKeyPath;
+
+  private OMMetadataManager omMetadataManager;
+
+  public OMRecoverLeaseRequest(OMRequest omRequest) {
+    super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    Preconditions.checkNotNull(recoverLeaseRequest);
+    volumeName = recoverLeaseRequest.getVolumeName();
+    bucketName = recoverLeaseRequest.getBucketName();
+    keyName = recoverLeaseRequest.getKeyName();
+  }
+
+  @Override
+  public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    String keyPath = recoverLeaseRequest.getKeyName();
+    normalizedKeyPath =
+        validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+        keyPath, getBucketLayout());
+
+    return getOmRequest().toBuilder()
+        .setRecoverLeaseRequest(
+            recoverLeaseRequest.toBuilder()
+                .setKeyName(normalizedKeyPath))
+        .build();
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+      long transactionLogIndex,
+      OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+    Preconditions.checkNotNull(recoverLeaseRequest);
+
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.VOLUME, volumeName);
+    auditMap.put(OzoneConsts.BUCKET, bucketName);
+    auditMap.put(OzoneConsts.KEY, keyName);
+
+    OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+        getOmRequest());
+
+    omMetadataManager = ozoneManager.getMetadataManager();
+    OMClientResponse omClientResponse = null;
+    IOException exception = null;
+    // increment metric
+    OMMetrics omMetrics = ozoneManager.getMetrics();
+
+    boolean acquiredLock = false;
+    try {
+      // check ACL
+      checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+          IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+      // acquire lock
+      acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+          volumeName, bucketName);
+
+      validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+      String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+          transactionLogIndex);
+
+      // Prepare response
+      boolean responseCode = true;
+      omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+              .setResponse(responseCode).build())
+          .setCmdType(RecoverLease);
+      omClientResponse =
+          new OMRecoverLeaseResponse(omResponse.build(), getBucketLayout(),
+              openKeyEntryName);
+      omMetrics.incNumRecoverLease();
+      LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+          bucketName, keyName);
+    } catch (IOException ex) {
+      LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+          volumeName, bucketName, keyName, ex);
+      exception = ex;
+      omMetrics.incNumRecoverLeaseFails();
+      omResponse.setCmdType(RecoverLease);
+      omClientResponse = new OMRecoverLeaseResponse(
+          createErrorOMResponse(omResponse, ex), getBucketLayout());
+    } finally {
+      if (omClientResponse != null) {
+        omClientResponse.setFlushFuture(
+            ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                transactionLogIndex));
+      }
+      if (acquiredLock) {
+        omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+            bucketName);
+      }
+    }
+
+    // Audit Log outside the lock
+    auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+        OMAction.RECOVER_LEASE, auditMap, exception,
+        getOmRequest().getUserInfo()));
+
+    return omClientResponse;
+  }
+
+  private String doWork(OzoneManager ozoneManager,
+      RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+      throws IOException {
+
+    final long volumeId = omMetadataManager.getVolumeId(volumeName);
+    final long bucketId = omMetadataManager.getBucketId(
+        volumeName, bucketName);
+    Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+    long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+        pathComponents, keyName, omMetadataManager,
+        "Cannot recover file : " + keyName
+            + " as parent directory doesn't exist");
+    String fileName = OzoneFSUtils.getFileName(keyName);
+    String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+        parentID, fileName);
+
+    OmKeyInfo keyInfo = getKey(dbFileKey);
+    if (keyInfo == null) {
+      throw new OMException("Key:" + keyName + " not found", KEY_NOT_FOUND);
+    }
+    String openKeyEntryName = getOpenFileEntryName(volumeId, bucketId,
+        parentID, fileName);
+    if (openKeyEntryName != null) {
+      checkFileState(keyInfo, openKeyEntryName);
+      commitKey(dbFileKey, keyInfo, ozoneManager, transactionLogIndex);
+      removeOpenKey(openKeyEntryName, transactionLogIndex);
+    }

Review Comment:
   Sure.



-- 
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] sumitagrawl commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1157013257


##########
hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java:
##########
@@ -505,6 +505,19 @@ String getOpenFileName(long volumeId, long bucketId,
    */
   String getRenameKey(String volume, String bucket, long objectID);
 
+  /**
+   * Returns DB key name of an open file in OM metadata store. Should be
+   * #open# prefix followed by actual leaf node name.
+   *
+   * @param volumeId       - ID of the volume
+   * @param bucketId       - ID of the bucket
+   * @param parentObjectId - parent object Id
+   * @param fileName       - file name
+   * @return DB directory key as String.
+   */
+  String getOpenFileNamePrefix(long volumeId, long bucketId,

Review Comment:
   We can use existing method org.apache.hadoop.ozone.om.OMMetadataManager#getOzonePathKey(long, long, long, java.lang.String)



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/file/OMRecoverLeaseResponse.java:
##########
@@ -0,0 +1,80 @@
+/**
+ * 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.ozone.om.response.file;
+
+import org.apache.hadoop.hdds.utils.db.BatchOperation;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.response.CleanupTableInfo;
+import org.apache.hadoop.ozone.om.response.key.OmKeyResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+
+import javax.annotation.Nonnull;
+import java.io.IOException;
+
+import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.FILE_TABLE;
+import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.OPEN_FILE_TABLE;
+
+/**
+ * Performs tasks for RecoverLease request responses.
+ */
+@CleanupTableInfo(cleanupTables = {FILE_TABLE, OPEN_FILE_TABLE})
+public class OMRecoverLeaseResponse extends OmKeyResponse {
+
+  private OmKeyInfo keyInfo;
+  private String dbFileKey;
+  private String openKeyName;
+  public OMRecoverLeaseResponse(@Nonnull OMResponse omResponse,
+      BucketLayout bucketLayout, OmKeyInfo keyInfo, String dbFileKey,
+      String openKeyName) {
+    super(omResponse, bucketLayout);
+    this.keyInfo = keyInfo;
+    this.dbFileKey = dbFileKey;
+    this.openKeyName = openKeyName;
+  }
+
+  /**
+   * For when the request is not successful.
+   * For a successful request, the other constructor should be used.
+   */
+  public OMRecoverLeaseResponse(@Nonnull OMResponse omResponse,
+      @Nonnull BucketLayout bucketLayout) {
+    super(omResponse, bucketLayout);
+    checkStatusNotOK();
+  }
+
+  @Override
+  protected void addToDBBatch(OMMetadataManager omMetadataManager,
+      BatchOperation batchOperation) throws IOException {
+    // Delete from OpenKey table
+    if (openKeyName != null) {
+      omMetadataManager.getOpenKeyTable(getBucketLayout()).deleteWithBatch(
+          batchOperation, openKeyName);
+    }
+    omMetadataManager.getKeyTable(BucketLayout.FILE_SYSTEM_OPTIMIZED)

Review Comment:
   If openKeyInfo is not present, we may not need to update keyTable again, its like no-ops kind of operation.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,281 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private OMMetadataManager omMetadataManager;
+
+  public OMRecoverLeaseRequest(OMRequest omRequest) {
+    super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    Preconditions.checkNotNull(recoverLeaseRequest);
+    volumeName = recoverLeaseRequest.getVolumeName();
+    bucketName = recoverLeaseRequest.getBucketName();
+    keyName = recoverLeaseRequest.getKeyName();
+  }
+
+  @Override
+  public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    String keyPath = recoverLeaseRequest.getKeyName();
+    String normalizedKeyPath =
+        validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+    return getOmRequest().toBuilder()
+        .setRecoverLeaseRequest(
+            recoverLeaseRequest.toBuilder()
+                .setKeyName(normalizedKeyPath))
+        .build();
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+      long transactionLogIndex,
+      OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+    Preconditions.checkNotNull(recoverLeaseRequest);
+
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.VOLUME, volumeName);
+    auditMap.put(OzoneConsts.BUCKET, bucketName);
+    auditMap.put(OzoneConsts.KEY, keyName);
+
+    OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+        getOmRequest());
+
+    omMetadataManager = ozoneManager.getMetadataManager();
+    OMClientResponse omClientResponse = null;
+    IOException exception = null;
+    // increment metric
+    OMMetrics omMetrics = ozoneManager.getMetrics();
+
+    boolean acquiredLock = false;
+    try {
+      // check ACL
+      checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+          IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+      // acquire lock
+      acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+          volumeName, bucketName);
+
+      validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+      String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+          transactionLogIndex);
+
+      // Prepare response
+      boolean responseCode = true;
+      omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+              .setResponse(responseCode).build())
+          .setCmdType(RecoverLease);
+      omClientResponse =
+          new OMRecoverLeaseResponse(omResponse.build(), getBucketLayout(),
+              openKeyEntryName);
+      omMetrics.incNumRecoverLease();
+      LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+          bucketName, keyName);
+    } catch (IOException ex) {
+      LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+          volumeName, bucketName, keyName, ex);
+      exception = ex;
+      omMetrics.incNumRecoverLeaseFails();
+      omResponse.setCmdType(RecoverLease);
+      omClientResponse = new OMRecoverLeaseResponse(
+          createErrorOMResponse(omResponse, ex), getBucketLayout());
+    } finally {
+      if (omClientResponse != null) {
+        omClientResponse.setFlushFuture(
+            ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                transactionLogIndex));
+      }
+      if (acquiredLock) {
+        omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+            bucketName);
+      }
+    }
+
+    // Audit Log outside the lock
+    auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+        OMAction.RECOVER_LEASE, auditMap, exception,
+        getOmRequest().getUserInfo()));
+
+    return omClientResponse;
+  }
+
+  private String doWork(OzoneManager ozoneManager,
+      RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+      throws IOException {
+
+    final long volumeId = omMetadataManager.getVolumeId(volumeName);
+    final long bucketId = omMetadataManager.getBucketId(
+        volumeName, bucketName);
+    Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+    long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+        pathComponents, keyName, omMetadataManager,
+        "Cannot recover file : " + keyName
+            + " as parent directory doesn't exist");
+    String fileName = OzoneFSUtils.getFileName(keyName);
+    String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+        parentID, fileName);
+
+    OmKeyInfo keyInfo = getKey(dbFileKey);

Review Comment:
   @jojochuang 
   This releaseRecovery is not supported as TestOMRecoverLeaseRequest.testRecoverOpenFile with assert KEY_NOT_FOUND.  Seems scope is only reaseRecovery of HSync file, please confirm if this is only scope.



-- 
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] szetszwo commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1156790568


##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java:
##########
@@ -983,4 +983,16 @@ EchoRPCResponse echoRPCReq(byte[] payloadReq,
                              int payloadSizeResp)
           throws IOException;
 
+
+  /**
+   * Start the lease recovery of a file.
+   *
+   * @param volumeName - The volume name.
+   * @param bucketName - The bucket name.
+   * @param keyName - The key user want to recover.
+   * @return true if the file is already closed
+   * @throws IOException if an error occurs
+   */
+  boolean recoverLease(String volumeName, String bucketName,
+                              String keyName) throws IOException;

Review Comment:
   The client related methods (this and many other methods) should be moved to `OzoneManagerClientProtocol`.  We could do it in a separated JIRA.



-- 
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] sumitagrawl commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1099680980


##########
hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java:
##########
@@ -472,6 +472,19 @@ String getOzonePathKey(long volumeId, long bucketId,
   String getOpenFileName(long volumeId, long bucketId,
                          long parentObjectId, String fileName, long id);
 
+  /**
+   * Returns DB key name of an open file in OM metadata store. Should be

Review Comment:
   Need update the comment as usecase, as prefix path till filename



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,264 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+    private static final Logger LOG =
+            LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+    RecoverLeaseRequest recoverLeaseRequest;
+
+    private String volumeName;
+    private String bucketName;
+    private String keyName;
+
+    private String normalizedKeyPath;
+
+    private OMMetadataManager omMetadataManager;
+
+    public OMRecoverLeaseRequest(OMRequest omRequest) {
+        super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+        RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+            .getRecoverLeaseRequest();
+        Preconditions.checkNotNull(recoverLeaseRequest);
+        volumeName = recoverLeaseRequest.getVolumeName();
+        bucketName = recoverLeaseRequest.getBucketName();
+        keyName = recoverLeaseRequest.getKeyName();
+    }
+
+    @Override
+    public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+        RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+                .getRecoverLeaseRequest();
+
+        String keyPath = recoverLeaseRequest.getKeyName();
+        normalizedKeyPath = validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+        return getOmRequest().toBuilder()
+            .setRecoverLeaseRequest(
+                recoverLeaseRequest.toBuilder()
+                    .setKeyName(normalizedKeyPath))
+            .build();
+    }
+
+    @Override
+    public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, long transactionLogIndex, OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+        RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+                .getRecoverLeaseRequest();
+        Preconditions.checkNotNull(recoverLeaseRequest);
+
+        Map<String, String> auditMap = new LinkedHashMap<>();
+        auditMap.put(OzoneConsts.VOLUME, volumeName);
+        auditMap.put(OzoneConsts.BUCKET, bucketName);
+        auditMap.put(OzoneConsts.KEY, keyName);
+
+        OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+                getOmRequest());
+
+        omMetadataManager = ozoneManager.getMetadataManager();
+        OMClientResponse omClientResponse = null;
+        IOException exception = null;
+        // increment metric
+        OMMetrics omMetrics = ozoneManager.getMetrics();
+
+        boolean acquiredLock = false;
+        try {
+            // check ACL
+            checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+                    IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+            // acquire lock
+            acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+                    volumeName, bucketName);
+
+            validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+            String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+                transactionLogIndex);
+
+            // Prepare response
+            boolean responseCode = true;
+            omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+                            .setResponse(responseCode).build())
+                    .setCmdType(RecoverLease);
+            omClientResponse = new OMRecoverLeaseResponse(omResponse.build(),
+                    getBucketLayout(), openKeyEntryName);
+            omMetrics.incNumRecoverLease();
+            LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+                bucketName, keyName);
+        } catch (IOException ex) {
+            LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+                volumeName, bucketName, keyName, ex);
+            exception = ex;
+            omMetrics.incNumRecoverLeaseFails();
+            omResponse.setCmdType(RecoverLease);
+            omClientResponse = new OMRecoverLeaseResponse(
+                createErrorOMResponse(omResponse, ex), getBucketLayout(), null);
+        } finally {
+            if (omClientResponse != null) {
+                omClientResponse.setFlushFuture(
+                        ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                                transactionLogIndex));
+            }
+            if (acquiredLock) {
+                omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+                        bucketName);
+            }
+        }
+
+        // Audit Log outside the lock
+        auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+                OMAction.RECOVER_LEASE, auditMap, exception,
+                getOmRequest().getUserInfo()));
+
+        return omClientResponse;
+    }
+
+    private String doWork(OzoneManager ozoneManager,
+        RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+        throws IOException {
+
+        //String dbOzoneKey =
+        //    omMetadataManager.getOzoneKey(volumeName, bucketName,
+        //        keyName);
+        final long volumeId = omMetadataManager.getVolumeId(volumeName);
+        final long bucketId = omMetadataManager.getBucketId(
+            volumeName, bucketName);
+        Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+        long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+            pathComponents, keyName, omMetadataManager,
+            "Cannot recover file : " + keyName
+                + " as parent directory doesn't exist");
+        String fileName = OzoneFSUtils.getFileName(keyName);
+        String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+            parentID, fileName);
+
+        OmKeyInfo keyInfo = getKey(dbFileKey);
+        String openKeyEntryName = getOpenFileEntryName(volumeId, bucketId, parentID, fileName);
+
+        checkFileState(keyInfo, openKeyEntryName);
+
+        commitKey(dbFileKey, keyInfo, ozoneManager, transactionLogIndex);
+
+        removeOpenKey(openKeyEntryName, transactionLogIndex);
+
+        return openKeyEntryName;
+    }
+
+    private OmKeyInfo getKey(String dbOzoneKey) throws IOException {
+        return omMetadataManager.getKeyTable(getBucketLayout()).get(dbOzoneKey);
+    }
+
+    private String getOpenFileEntryName(long volumeId, long bucketId,
+        long parentObjectId, String fileName) throws IOException {
+        String openFileEntryName = "";
+        String dbOpenKeyPrefix =
+            omMetadataManager.getOpenFileNamePrefix(volumeId, bucketId, parentObjectId, fileName);
+        try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
+            iter = omMetadataManager.getOpenKeyTable(getBucketLayout())
+            .iterator(dbOpenKeyPrefix)) {
+
+            // TODO: verify that there should only be one entry for the open key
+            while (iter.hasNext()) {
+                //OmKeyInfo keyInfo = iter.next().getValue();

Review Comment:
   can remove commented code



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,264 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+    private static final Logger LOG =
+            LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+    RecoverLeaseRequest recoverLeaseRequest;
+
+    private String volumeName;
+    private String bucketName;
+    private String keyName;
+
+    private String normalizedKeyPath;
+
+    private OMMetadataManager omMetadataManager;
+
+    public OMRecoverLeaseRequest(OMRequest omRequest) {
+        super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+        RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+            .getRecoverLeaseRequest();
+        Preconditions.checkNotNull(recoverLeaseRequest);
+        volumeName = recoverLeaseRequest.getVolumeName();
+        bucketName = recoverLeaseRequest.getBucketName();
+        keyName = recoverLeaseRequest.getKeyName();
+    }
+
+    @Override
+    public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+        RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+                .getRecoverLeaseRequest();
+
+        String keyPath = recoverLeaseRequest.getKeyName();
+        normalizedKeyPath = validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+        return getOmRequest().toBuilder()
+            .setRecoverLeaseRequest(
+                recoverLeaseRequest.toBuilder()
+                    .setKeyName(normalizedKeyPath))
+            .build();
+    }
+
+    @Override
+    public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, long transactionLogIndex, OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+        RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+                .getRecoverLeaseRequest();
+        Preconditions.checkNotNull(recoverLeaseRequest);
+
+        Map<String, String> auditMap = new LinkedHashMap<>();
+        auditMap.put(OzoneConsts.VOLUME, volumeName);
+        auditMap.put(OzoneConsts.BUCKET, bucketName);
+        auditMap.put(OzoneConsts.KEY, keyName);
+
+        OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+                getOmRequest());
+
+        omMetadataManager = ozoneManager.getMetadataManager();
+        OMClientResponse omClientResponse = null;
+        IOException exception = null;
+        // increment metric
+        OMMetrics omMetrics = ozoneManager.getMetrics();
+
+        boolean acquiredLock = false;
+        try {
+            // check ACL
+            checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+                    IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+            // acquire lock
+            acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+                    volumeName, bucketName);
+
+            validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+            String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+                transactionLogIndex);
+
+            // Prepare response
+            boolean responseCode = true;
+            omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+                            .setResponse(responseCode).build())
+                    .setCmdType(RecoverLease);
+            omClientResponse = new OMRecoverLeaseResponse(omResponse.build(),
+                    getBucketLayout(), openKeyEntryName);
+            omMetrics.incNumRecoverLease();
+            LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+                bucketName, keyName);
+        } catch (IOException ex) {
+            LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+                volumeName, bucketName, keyName, ex);
+            exception = ex;
+            omMetrics.incNumRecoverLeaseFails();
+            omResponse.setCmdType(RecoverLease);
+            omClientResponse = new OMRecoverLeaseResponse(
+                createErrorOMResponse(omResponse, ex), getBucketLayout(), null);
+        } finally {
+            if (omClientResponse != null) {
+                omClientResponse.setFlushFuture(
+                        ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                                transactionLogIndex));
+            }
+            if (acquiredLock) {
+                omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+                        bucketName);
+            }
+        }
+
+        // Audit Log outside the lock
+        auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+                OMAction.RECOVER_LEASE, auditMap, exception,
+                getOmRequest().getUserInfo()));
+
+        return omClientResponse;
+    }
+
+    private String doWork(OzoneManager ozoneManager,
+        RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+        throws IOException {
+
+        //String dbOzoneKey =
+        //    omMetadataManager.getOzoneKey(volumeName, bucketName,
+        //        keyName);
+        final long volumeId = omMetadataManager.getVolumeId(volumeName);
+        final long bucketId = omMetadataManager.getBucketId(
+            volumeName, bucketName);
+        Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+        long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+            pathComponents, keyName, omMetadataManager,
+            "Cannot recover file : " + keyName
+                + " as parent directory doesn't exist");
+        String fileName = OzoneFSUtils.getFileName(keyName);
+        String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+            parentID, fileName);
+
+        OmKeyInfo keyInfo = getKey(dbFileKey);

Review Comment:
   As my understanding, if a key is present in openKeyTable, entry will not be there in keyTable, please recheck



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/file/OMRecoverLeaseResponse.java:
##########
@@ -0,0 +1,44 @@
+package org.apache.hadoop.ozone.om.response.file;
+
+import org.apache.hadoop.hdds.utils.db.BatchOperation;
+import org.apache.hadoop.ozone.OmUtils;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo;
+import org.apache.hadoop.ozone.om.response.CleanupTableInfo;
+import org.apache.hadoop.ozone.om.response.key.OmKeyResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import java.io.IOException;
+
+import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.FILE_TABLE;
+import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.OPEN_FILE_TABLE;
+
+@CleanupTableInfo(cleanupTables = {FILE_TABLE, OPEN_FILE_TABLE})
+public class OMRecoverLeaseResponse extends OmKeyResponse {
+
+    private String openKeyName;
+    public OMRecoverLeaseResponse(@Nonnull OMResponse omResponse,
+        BucketLayout bucketLayout, String openKeyName) {
+        super(omResponse, bucketLayout);
+        checkStatusNotOK();
+        this.openKeyName = openKeyName;
+    }
+
+    @Override
+    protected void addToDBBatch(OMMetadataManager omMetadataManager,
+                                BatchOperation batchOperation) throws IOException {
+        // Delete from OpenKey table
+        omMetadataManager.getOpenKeyTable(getBucketLayout())

Review Comment:
   If some blocks occupied and present in openKeyTable, those cleanup needs to be handled



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,264 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+    private static final Logger LOG =
+            LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+    RecoverLeaseRequest recoverLeaseRequest;
+
+    private String volumeName;
+    private String bucketName;
+    private String keyName;
+
+    private String normalizedKeyPath;
+
+    private OMMetadataManager omMetadataManager;
+
+    public OMRecoverLeaseRequest(OMRequest omRequest) {
+        super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+        RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+            .getRecoverLeaseRequest();
+        Preconditions.checkNotNull(recoverLeaseRequest);
+        volumeName = recoverLeaseRequest.getVolumeName();
+        bucketName = recoverLeaseRequest.getBucketName();
+        keyName = recoverLeaseRequest.getKeyName();
+    }
+
+    @Override
+    public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+        RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+                .getRecoverLeaseRequest();
+
+        String keyPath = recoverLeaseRequest.getKeyName();
+        normalizedKeyPath = validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+        return getOmRequest().toBuilder()
+            .setRecoverLeaseRequest(
+                recoverLeaseRequest.toBuilder()
+                    .setKeyName(normalizedKeyPath))
+            .build();
+    }
+
+    @Override
+    public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, long transactionLogIndex, OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+        RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+                .getRecoverLeaseRequest();
+        Preconditions.checkNotNull(recoverLeaseRequest);
+
+        Map<String, String> auditMap = new LinkedHashMap<>();
+        auditMap.put(OzoneConsts.VOLUME, volumeName);
+        auditMap.put(OzoneConsts.BUCKET, bucketName);
+        auditMap.put(OzoneConsts.KEY, keyName);
+
+        OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+                getOmRequest());
+
+        omMetadataManager = ozoneManager.getMetadataManager();
+        OMClientResponse omClientResponse = null;
+        IOException exception = null;
+        // increment metric
+        OMMetrics omMetrics = ozoneManager.getMetrics();
+
+        boolean acquiredLock = false;
+        try {
+            // check ACL
+            checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+                    IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+            // acquire lock
+            acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+                    volumeName, bucketName);
+
+            validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+            String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+                transactionLogIndex);
+
+            // Prepare response
+            boolean responseCode = true;
+            omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+                            .setResponse(responseCode).build())
+                    .setCmdType(RecoverLease);
+            omClientResponse = new OMRecoverLeaseResponse(omResponse.build(),
+                    getBucketLayout(), openKeyEntryName);
+            omMetrics.incNumRecoverLease();
+            LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+                bucketName, keyName);
+        } catch (IOException ex) {
+            LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+                volumeName, bucketName, keyName, ex);
+            exception = ex;
+            omMetrics.incNumRecoverLeaseFails();
+            omResponse.setCmdType(RecoverLease);
+            omClientResponse = new OMRecoverLeaseResponse(
+                createErrorOMResponse(omResponse, ex), getBucketLayout(), null);
+        } finally {
+            if (omClientResponse != null) {
+                omClientResponse.setFlushFuture(
+                        ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                                transactionLogIndex));
+            }
+            if (acquiredLock) {
+                omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+                        bucketName);
+            }
+        }
+
+        // Audit Log outside the lock
+        auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+                OMAction.RECOVER_LEASE, auditMap, exception,
+                getOmRequest().getUserInfo()));
+
+        return omClientResponse;
+    }
+
+    private String doWork(OzoneManager ozoneManager,
+        RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+        throws IOException {
+
+        //String dbOzoneKey =
+        //    omMetadataManager.getOzoneKey(volumeName, bucketName,
+        //        keyName);
+        final long volumeId = omMetadataManager.getVolumeId(volumeName);
+        final long bucketId = omMetadataManager.getBucketId(
+            volumeName, bucketName);
+        Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+        long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+            pathComponents, keyName, omMetadataManager,
+            "Cannot recover file : " + keyName
+                + " as parent directory doesn't exist");
+        String fileName = OzoneFSUtils.getFileName(keyName);
+        String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+            parentID, fileName);
+
+        OmKeyInfo keyInfo = getKey(dbFileKey);
+        String openKeyEntryName = getOpenFileEntryName(volumeId, bucketId, parentID, fileName);
+
+        checkFileState(keyInfo, openKeyEntryName);
+
+        commitKey(dbFileKey, keyInfo, ozoneManager, transactionLogIndex);
+
+        removeOpenKey(openKeyEntryName, transactionLogIndex);
+
+        return openKeyEntryName;
+    }
+
+    private OmKeyInfo getKey(String dbOzoneKey) throws IOException {
+        return omMetadataManager.getKeyTable(getBucketLayout()).get(dbOzoneKey);
+    }
+
+    private String getOpenFileEntryName(long volumeId, long bucketId,
+        long parentObjectId, String fileName) throws IOException {
+        String openFileEntryName = "";
+        String dbOpenKeyPrefix =
+            omMetadataManager.getOpenFileNamePrefix(volumeId, bucketId, parentObjectId, fileName);
+        try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
+            iter = omMetadataManager.getOpenKeyTable(getBucketLayout())
+            .iterator(dbOpenKeyPrefix)) {
+
+            // TODO: verify that there should only be one entry for the open key

Review Comment:
   I think multiple file can be uploaded parallel and commit is successful whoever commits first, check this part.
   
   Also cleanup is done for any client, Do any waiting period is required? or cleanup immediately is fine as per HDFS behavior ?



##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java:
##########
@@ -321,6 +321,7 @@ public static boolean isReadOnly(
     case TenantAssignAdmin:
     case TenantRevokeAdmin:
     case SetRangerServiceVersion:
+    case RecoverLease:

Review Comment:
   Ozone do not have any acquireLease kind of interface, this name will be confusing wrt ozone. It can be RecoverOpenKey. 



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,264 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+    private static final Logger LOG =
+            LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+    RecoverLeaseRequest recoverLeaseRequest;
+
+    private String volumeName;
+    private String bucketName;
+    private String keyName;
+
+    private String normalizedKeyPath;
+
+    private OMMetadataManager omMetadataManager;
+
+    public OMRecoverLeaseRequest(OMRequest omRequest) {
+        super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+        RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+            .getRecoverLeaseRequest();
+        Preconditions.checkNotNull(recoverLeaseRequest);
+        volumeName = recoverLeaseRequest.getVolumeName();
+        bucketName = recoverLeaseRequest.getBucketName();
+        keyName = recoverLeaseRequest.getKeyName();
+    }
+
+    @Override
+    public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+        RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+                .getRecoverLeaseRequest();
+
+        String keyPath = recoverLeaseRequest.getKeyName();
+        normalizedKeyPath = validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+        return getOmRequest().toBuilder()
+            .setRecoverLeaseRequest(
+                recoverLeaseRequest.toBuilder()
+                    .setKeyName(normalizedKeyPath))
+            .build();
+    }
+
+    @Override
+    public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, long transactionLogIndex, OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+        RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+                .getRecoverLeaseRequest();
+        Preconditions.checkNotNull(recoverLeaseRequest);
+
+        Map<String, String> auditMap = new LinkedHashMap<>();
+        auditMap.put(OzoneConsts.VOLUME, volumeName);
+        auditMap.put(OzoneConsts.BUCKET, bucketName);
+        auditMap.put(OzoneConsts.KEY, keyName);
+
+        OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+                getOmRequest());
+
+        omMetadataManager = ozoneManager.getMetadataManager();
+        OMClientResponse omClientResponse = null;
+        IOException exception = null;
+        // increment metric
+        OMMetrics omMetrics = ozoneManager.getMetrics();
+
+        boolean acquiredLock = false;
+        try {
+            // check ACL
+            checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+                    IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+            // acquire lock
+            acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+                    volumeName, bucketName);
+
+            validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+            String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+                transactionLogIndex);
+
+            // Prepare response
+            boolean responseCode = true;
+            omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+                            .setResponse(responseCode).build())
+                    .setCmdType(RecoverLease);
+            omClientResponse = new OMRecoverLeaseResponse(omResponse.build(),
+                    getBucketLayout(), openKeyEntryName);
+            omMetrics.incNumRecoverLease();
+            LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+                bucketName, keyName);
+        } catch (IOException ex) {
+            LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+                volumeName, bucketName, keyName, ex);
+            exception = ex;
+            omMetrics.incNumRecoverLeaseFails();
+            omResponse.setCmdType(RecoverLease);
+            omClientResponse = new OMRecoverLeaseResponse(
+                createErrorOMResponse(omResponse, ex), getBucketLayout(), null);
+        } finally {
+            if (omClientResponse != null) {
+                omClientResponse.setFlushFuture(
+                        ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                                transactionLogIndex));
+            }
+            if (acquiredLock) {
+                omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+                        bucketName);
+            }
+        }
+
+        // Audit Log outside the lock
+        auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+                OMAction.RECOVER_LEASE, auditMap, exception,
+                getOmRequest().getUserInfo()));
+
+        return omClientResponse;
+    }
+
+    private String doWork(OzoneManager ozoneManager,
+        RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+        throws IOException {
+
+        //String dbOzoneKey =
+        //    omMetadataManager.getOzoneKey(volumeName, bucketName,
+        //        keyName);
+        final long volumeId = omMetadataManager.getVolumeId(volumeName);
+        final long bucketId = omMetadataManager.getBucketId(
+            volumeName, bucketName);
+        Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+        long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+            pathComponents, keyName, omMetadataManager,
+            "Cannot recover file : " + keyName
+                + " as parent directory doesn't exist");
+        String fileName = OzoneFSUtils.getFileName(keyName);
+        String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+            parentID, fileName);
+
+        OmKeyInfo keyInfo = getKey(dbFileKey);
+        String openKeyEntryName = getOpenFileEntryName(volumeId, bucketId, parentID, fileName);
+
+        checkFileState(keyInfo, openKeyEntryName);
+
+        commitKey(dbFileKey, keyInfo, ozoneManager, transactionLogIndex);

Review Comment:
   I am not finding usages of adding keyInfo to cache as its cleaned up in flush



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java:
##########
@@ -216,7 +218,17 @@ public static OMClientRequest createClientRequest(OMRequest omRequest,
             omRequest.getDeleteOpenKeysRequest().getBucketLayout());
       }
       return new OMOpenKeysDeleteRequest(omRequest, bktLayout);
-
+    case RecoverLease: {
+      volumeName = omRequest.getRecoverLeaseRequest().getVolumeName();
+      bucketName = omRequest.getRecoverLeaseRequest().getBucketName();
+      bucketLayout =
+              getBucketLayout(ozoneManager.getMetadataManager(), volumeName, bucketName);
+      if (bucketLayout != BucketLayout.FILE_SYSTEM_OPTIMIZED) {

Review Comment:
   ozone also have object store, check if included as part of this to cleanup openKeyTable



-- 
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] jojochuang commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1154731027


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,281 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private OMMetadataManager omMetadataManager;
+
+  public OMRecoverLeaseRequest(OMRequest omRequest) {
+    super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    Preconditions.checkNotNull(recoverLeaseRequest);
+    volumeName = recoverLeaseRequest.getVolumeName();
+    bucketName = recoverLeaseRequest.getBucketName();
+    keyName = recoverLeaseRequest.getKeyName();
+  }
+
+  @Override
+  public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    String keyPath = recoverLeaseRequest.getKeyName();
+    String normalizedKeyPath =
+        validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+    return getOmRequest().toBuilder()
+        .setRecoverLeaseRequest(
+            recoverLeaseRequest.toBuilder()
+                .setKeyName(normalizedKeyPath))
+        .build();
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+      long transactionLogIndex,
+      OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+    Preconditions.checkNotNull(recoverLeaseRequest);
+
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.VOLUME, volumeName);
+    auditMap.put(OzoneConsts.BUCKET, bucketName);
+    auditMap.put(OzoneConsts.KEY, keyName);
+
+    OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+        getOmRequest());
+
+    omMetadataManager = ozoneManager.getMetadataManager();
+    OMClientResponse omClientResponse = null;
+    IOException exception = null;
+    // increment metric
+    OMMetrics omMetrics = ozoneManager.getMetrics();
+
+    boolean acquiredLock = false;
+    try {
+      // check ACL
+      checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+          IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+      // acquire lock
+      acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+          volumeName, bucketName);
+
+      validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+      String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+          transactionLogIndex);
+
+      // Prepare response
+      boolean responseCode = true;
+      omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+              .setResponse(responseCode).build())
+          .setCmdType(RecoverLease);
+      omClientResponse =
+          new OMRecoverLeaseResponse(omResponse.build(), getBucketLayout(),
+              openKeyEntryName);
+      omMetrics.incNumRecoverLease();
+      LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+          bucketName, keyName);
+    } catch (IOException ex) {
+      LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+          volumeName, bucketName, keyName, ex);
+      exception = ex;
+      omMetrics.incNumRecoverLeaseFails();
+      omResponse.setCmdType(RecoverLease);
+      omClientResponse = new OMRecoverLeaseResponse(
+          createErrorOMResponse(omResponse, ex), getBucketLayout());
+    } finally {
+      if (omClientResponse != null) {
+        omClientResponse.setFlushFuture(
+            ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                transactionLogIndex));
+      }
+      if (acquiredLock) {
+        omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+            bucketName);
+      }
+    }
+
+    // Audit Log outside the lock
+    auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+        OMAction.RECOVER_LEASE, auditMap, exception,
+        getOmRequest().getUserInfo()));
+
+    return omClientResponse;
+  }
+
+  private String doWork(OzoneManager ozoneManager,
+      RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+      throws IOException {
+
+    final long volumeId = omMetadataManager.getVolumeId(volumeName);
+    final long bucketId = omMetadataManager.getBucketId(
+        volumeName, bucketName);
+    Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+    long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+        pathComponents, keyName, omMetadataManager,
+        "Cannot recover file : " + keyName
+            + " as parent directory doesn't exist");
+    String fileName = OzoneFSUtils.getFileName(keyName);
+    String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+        parentID, fileName);
+
+    OmKeyInfo keyInfo = getKey(dbFileKey);
+    if (keyInfo == null) {
+      throw new OMException("Key:" + keyName + " not found", KEY_NOT_FOUND);
+    }
+    final String clientId = keyInfo.getMetadata().get(
+        OzoneConsts.HSYNC_CLIENT_ID);
+    if (clientId == null) {
+      LOG.warn("Key:" + keyName + " is closed");

Review Comment:
   Yes it should return immediately. However it should not throw an exception as it would break HBase.



-- 
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] szetszwo commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1158094564


##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java:
##########
@@ -983,4 +983,16 @@ EchoRPCResponse echoRPCReq(byte[] payloadReq,
                              int payloadSizeResp)
           throws IOException;
 
+
+  /**
+   * Start the lease recovery of a file.
+   *
+   * @param volumeName - The volume name.
+   * @param bucketName - The bucket name.
+   * @param keyName - The key user want to recover.
+   * @return true if the file is already closed
+   * @throws IOException if an error occurs
+   */
+  boolean recoverLease(String volumeName, String bucketName,
+                              String keyName) throws IOException;

Review Comment:
   Let's keep it as-is and do it separately.  It will be an easy but big change.



-- 
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] szetszwo commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1145733229


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,278 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private String normalizedKeyPath;

Review Comment:
   It should be a local variable.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,278 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private String normalizedKeyPath;
+
+  private OMMetadataManager omMetadataManager;
+
+  public OMRecoverLeaseRequest(OMRequest omRequest) {
+    super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    Preconditions.checkNotNull(recoverLeaseRequest);
+    volumeName = recoverLeaseRequest.getVolumeName();
+    bucketName = recoverLeaseRequest.getBucketName();
+    keyName = recoverLeaseRequest.getKeyName();
+  }
+
+  @Override
+  public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    String keyPath = recoverLeaseRequest.getKeyName();
+    normalizedKeyPath =
+        validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+        keyPath, getBucketLayout());
+
+    return getOmRequest().toBuilder()
+        .setRecoverLeaseRequest(
+            recoverLeaseRequest.toBuilder()
+                .setKeyName(normalizedKeyPath))
+        .build();
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+      long transactionLogIndex,
+      OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+    Preconditions.checkNotNull(recoverLeaseRequest);
+
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.VOLUME, volumeName);
+    auditMap.put(OzoneConsts.BUCKET, bucketName);
+    auditMap.put(OzoneConsts.KEY, keyName);
+
+    OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+        getOmRequest());
+
+    omMetadataManager = ozoneManager.getMetadataManager();
+    OMClientResponse omClientResponse = null;
+    IOException exception = null;
+    // increment metric
+    OMMetrics omMetrics = ozoneManager.getMetrics();
+
+    boolean acquiredLock = false;
+    try {
+      // check ACL
+      checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+          IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+      // acquire lock
+      acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+          volumeName, bucketName);
+
+      validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+      String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+          transactionLogIndex);
+
+      // Prepare response
+      boolean responseCode = true;
+      omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+              .setResponse(responseCode).build())
+          .setCmdType(RecoverLease);
+      omClientResponse =
+          new OMRecoverLeaseResponse(omResponse.build(), getBucketLayout(),
+              openKeyEntryName);
+      omMetrics.incNumRecoverLease();
+      LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+          bucketName, keyName);
+    } catch (IOException ex) {
+      LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+          volumeName, bucketName, keyName, ex);
+      exception = ex;
+      omMetrics.incNumRecoverLeaseFails();
+      omResponse.setCmdType(RecoverLease);
+      omClientResponse = new OMRecoverLeaseResponse(
+          createErrorOMResponse(omResponse, ex), getBucketLayout());
+    } finally {
+      if (omClientResponse != null) {
+        omClientResponse.setFlushFuture(
+            ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                transactionLogIndex));
+      }
+      if (acquiredLock) {
+        omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+            bucketName);
+      }
+    }
+
+    // Audit Log outside the lock
+    auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+        OMAction.RECOVER_LEASE, auditMap, exception,
+        getOmRequest().getUserInfo()));
+
+    return omClientResponse;
+  }
+
+  private String doWork(OzoneManager ozoneManager,
+      RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+      throws IOException {
+
+    final long volumeId = omMetadataManager.getVolumeId(volumeName);
+    final long bucketId = omMetadataManager.getBucketId(
+        volumeName, bucketName);
+    Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+    long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+        pathComponents, keyName, omMetadataManager,
+        "Cannot recover file : " + keyName
+            + " as parent directory doesn't exist");
+    String fileName = OzoneFSUtils.getFileName(keyName);
+    String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+        parentID, fileName);
+
+    OmKeyInfo keyInfo = getKey(dbFileKey);
+    if (keyInfo == null) {
+      throw new OMException("Key:" + keyName + " not found", KEY_NOT_FOUND);
+    }
+    String openKeyEntryName = getOpenFileEntryName(volumeId, bucketId,
+        parentID, fileName);
+    if (openKeyEntryName != null) {
+      checkFileState(keyInfo, openKeyEntryName);
+      commitKey(dbFileKey, keyInfo, ozoneManager, transactionLogIndex);
+      removeOpenKey(openKeyEntryName, transactionLogIndex);
+    }

Review Comment:
   Get `clientId` from metedata, i.e.
   ```java
       OmKeyInfo keyInfo = omMetadataManager.getKeyTable(getBucketLayout()).get(dbFileKey);
       if (keyInfo == null) {
         throw new OMException("Key:" + keyName + " not found", KEY_NOT_FOUND);
       }
       final String clientId = keyInfo.getMetadata().get(
           OzoneConsts.HSYNC_CLIENT_ID);
       if (clientId == null) {
         throw new OMException("Key:" + keyName + " is closed", KEY_ALREADY_EXISTS);
       }
       final String openKeyEntryName = omMetadataManager.getOpenFileName(
           volumeId, bucketId, parentID, fileName, Long.parseLong(clientId));
   
       checkFileState(keyInfo, openKeyEntryName);
       commitKey(dbFileKey, keyInfo, ozoneManager, transactionLogIndex);
       removeOpenKey(openKeyEntryName, transactionLogIndex, omMetadataManager);
   
       return openKeyEntryName;
   ```



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java:
##########
@@ -416,4 +416,27 @@ public static OMRequest blockCommitKeyWithBucketLayoutFromOldClient(
     }
     return req;
   }
+
+  @RequestFeatureValidator(
+      conditions = ValidationCondition.CLUSTER_NEEDS_FINALIZATION,
+      processingPhase = RequestProcessingPhase.PRE_PROCESS,
+      requestType = Type.CommitKey
+  )
+  public static OMRequest disallowCreateBucketWithECReplicationConfig(

Review Comment:
   Rename the method name.



-- 
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] jojochuang commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1157575016


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/file/OMRecoverLeaseResponse.java:
##########
@@ -0,0 +1,80 @@
+/**
+ * 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.ozone.om.response.file;
+
+import org.apache.hadoop.hdds.utils.db.BatchOperation;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.response.CleanupTableInfo;
+import org.apache.hadoop.ozone.om.response.key.OmKeyResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+
+import javax.annotation.Nonnull;
+import java.io.IOException;
+
+import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.FILE_TABLE;
+import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.OPEN_FILE_TABLE;
+
+/**
+ * Performs tasks for RecoverLease request responses.
+ */
+@CleanupTableInfo(cleanupTables = {FILE_TABLE, OPEN_FILE_TABLE})
+public class OMRecoverLeaseResponse extends OmKeyResponse {
+
+  private OmKeyInfo keyInfo;
+  private String dbFileKey;
+  private String openKeyName;
+  public OMRecoverLeaseResponse(@Nonnull OMResponse omResponse,
+      BucketLayout bucketLayout, OmKeyInfo keyInfo, String dbFileKey,
+      String openKeyName) {
+    super(omResponse, bucketLayout);
+    this.keyInfo = keyInfo;
+    this.dbFileKey = dbFileKey;
+    this.openKeyName = openKeyName;
+  }
+
+  /**
+   * For when the request is not successful.
+   * For a successful request, the other constructor should be used.
+   */
+  public OMRecoverLeaseResponse(@Nonnull OMResponse omResponse,
+      @Nonnull BucketLayout bucketLayout) {
+    super(omResponse, bucketLayout);
+    checkStatusNotOK();
+  }
+
+  @Override
+  protected void addToDBBatch(OMMetadataManager omMetadataManager,
+      BatchOperation batchOperation) throws IOException {
+    // Delete from OpenKey table
+    if (openKeyName != null) {
+      omMetadataManager.getOpenKeyTable(getBucketLayout()).deleteWithBatch(
+          batchOperation, openKeyName);
+    }
+    omMetadataManager.getKeyTable(BucketLayout.FILE_SYSTEM_OPTIMIZED)

Review Comment:
   > This releaseRecovery is not supported as TestOMRecoverLeaseRequest.testRecoverOpenFile with assert KEY_NOT_FOUND. Seems scope is only reaseRecovery of HSync file, please confirm if this is only scope.
   
   Yes the current implementation tries to make the behavior backward compatible with the existing Ozone behavior, where a file that is open for write is invisible in the namespace. So you're not supposed to be able to recover the lease successfully.
   
   I'm totally open to the idea of changing the behavior such that it is more aligned with HDFS behavior (e.g. once a file is opened it is visible in the namespace) but that need to open up in a separate jira for discussion.



-- 
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] jojochuang commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1119351531


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,264 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+    private static final Logger LOG =
+            LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+    RecoverLeaseRequest recoverLeaseRequest;
+
+    private String volumeName;
+    private String bucketName;
+    private String keyName;
+
+    private String normalizedKeyPath;
+
+    private OMMetadataManager omMetadataManager;
+
+    public OMRecoverLeaseRequest(OMRequest omRequest) {
+        super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+        RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+            .getRecoverLeaseRequest();
+        Preconditions.checkNotNull(recoverLeaseRequest);
+        volumeName = recoverLeaseRequest.getVolumeName();
+        bucketName = recoverLeaseRequest.getBucketName();
+        keyName = recoverLeaseRequest.getKeyName();
+    }
+
+    @Override
+    public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+        RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+                .getRecoverLeaseRequest();
+
+        String keyPath = recoverLeaseRequest.getKeyName();
+        normalizedKeyPath = validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+        return getOmRequest().toBuilder()
+            .setRecoverLeaseRequest(
+                recoverLeaseRequest.toBuilder()
+                    .setKeyName(normalizedKeyPath))
+            .build();
+    }
+
+    @Override
+    public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, long transactionLogIndex, OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+        RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+                .getRecoverLeaseRequest();
+        Preconditions.checkNotNull(recoverLeaseRequest);
+
+        Map<String, String> auditMap = new LinkedHashMap<>();
+        auditMap.put(OzoneConsts.VOLUME, volumeName);
+        auditMap.put(OzoneConsts.BUCKET, bucketName);
+        auditMap.put(OzoneConsts.KEY, keyName);
+
+        OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+                getOmRequest());
+
+        omMetadataManager = ozoneManager.getMetadataManager();
+        OMClientResponse omClientResponse = null;
+        IOException exception = null;
+        // increment metric
+        OMMetrics omMetrics = ozoneManager.getMetrics();
+
+        boolean acquiredLock = false;
+        try {
+            // check ACL
+            checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+                    IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+            // acquire lock
+            acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+                    volumeName, bucketName);
+
+            validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+            String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+                transactionLogIndex);
+
+            // Prepare response
+            boolean responseCode = true;
+            omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+                            .setResponse(responseCode).build())
+                    .setCmdType(RecoverLease);
+            omClientResponse = new OMRecoverLeaseResponse(omResponse.build(),
+                    getBucketLayout(), openKeyEntryName);
+            omMetrics.incNumRecoverLease();
+            LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+                bucketName, keyName);
+        } catch (IOException ex) {
+            LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+                volumeName, bucketName, keyName, ex);
+            exception = ex;
+            omMetrics.incNumRecoverLeaseFails();
+            omResponse.setCmdType(RecoverLease);
+            omClientResponse = new OMRecoverLeaseResponse(
+                createErrorOMResponse(omResponse, ex), getBucketLayout(), null);
+        } finally {
+            if (omClientResponse != null) {
+                omClientResponse.setFlushFuture(
+                        ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                                transactionLogIndex));
+            }
+            if (acquiredLock) {
+                omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+                        bucketName);
+            }
+        }
+
+        // Audit Log outside the lock
+        auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+                OMAction.RECOVER_LEASE, auditMap, exception,
+                getOmRequest().getUserInfo()));
+
+        return omClientResponse;
+    }
+
+    private String doWork(OzoneManager ozoneManager,
+        RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+        throws IOException {
+
+        //String dbOzoneKey =
+        //    omMetadataManager.getOzoneKey(volumeName, bucketName,
+        //        keyName);
+        final long volumeId = omMetadataManager.getVolumeId(volumeName);
+        final long bucketId = omMetadataManager.getBucketId(
+            volumeName, bucketName);
+        Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+        long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+            pathComponents, keyName, omMetadataManager,
+            "Cannot recover file : " + keyName
+                + " as parent directory doesn't exist");
+        String fileName = OzoneFSUtils.getFileName(keyName);
+        String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+            parentID, fileName);
+
+        OmKeyInfo keyInfo = getKey(dbFileKey);

Review Comment:
   once hsync is called on a file, the file will present in both tables.



-- 
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] jojochuang commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1119354584


##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java:
##########
@@ -321,6 +321,7 @@ public static boolean isReadOnly(
     case TenantAssignAdmin:
     case TenantRevokeAdmin:
     case SetRangerServiceVersion:
+    case RecoverLease:

Review Comment:
   The existing mechanisms in Ozone is as if the writer holds a 7-day lease which cannot be renewed. We plan to support full, HDFS-like lease where default lease is short and can be renewed.



-- 
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] jojochuang commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1154817125


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,281 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private OMMetadataManager omMetadataManager;
+
+  public OMRecoverLeaseRequest(OMRequest omRequest) {
+    super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    Preconditions.checkNotNull(recoverLeaseRequest);
+    volumeName = recoverLeaseRequest.getVolumeName();
+    bucketName = recoverLeaseRequest.getBucketName();
+    keyName = recoverLeaseRequest.getKeyName();
+  }
+
+  @Override
+  public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    String keyPath = recoverLeaseRequest.getKeyName();
+    String normalizedKeyPath =
+        validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+    return getOmRequest().toBuilder()
+        .setRecoverLeaseRequest(
+            recoverLeaseRequest.toBuilder()
+                .setKeyName(normalizedKeyPath))
+        .build();
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+      long transactionLogIndex,
+      OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+    Preconditions.checkNotNull(recoverLeaseRequest);
+
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.VOLUME, volumeName);
+    auditMap.put(OzoneConsts.BUCKET, bucketName);
+    auditMap.put(OzoneConsts.KEY, keyName);
+
+    OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+        getOmRequest());
+
+    omMetadataManager = ozoneManager.getMetadataManager();
+    OMClientResponse omClientResponse = null;
+    IOException exception = null;
+    // increment metric
+    OMMetrics omMetrics = ozoneManager.getMetrics();
+
+    boolean acquiredLock = false;
+    try {
+      // check ACL
+      checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+          IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+      // acquire lock
+      acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+          volumeName, bucketName);
+
+      validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+      String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+          transactionLogIndex);
+
+      // Prepare response
+      boolean responseCode = true;
+      omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+              .setResponse(responseCode).build())
+          .setCmdType(RecoverLease);
+      omClientResponse =
+          new OMRecoverLeaseResponse(omResponse.build(), getBucketLayout(),
+              openKeyEntryName);
+      omMetrics.incNumRecoverLease();
+      LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+          bucketName, keyName);
+    } catch (IOException ex) {
+      LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+          volumeName, bucketName, keyName, ex);
+      exception = ex;
+      omMetrics.incNumRecoverLeaseFails();
+      omResponse.setCmdType(RecoverLease);
+      omClientResponse = new OMRecoverLeaseResponse(
+          createErrorOMResponse(omResponse, ex), getBucketLayout());
+    } finally {
+      if (omClientResponse != null) {
+        omClientResponse.setFlushFuture(
+            ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                transactionLogIndex));
+      }
+      if (acquiredLock) {
+        omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+            bucketName);
+      }
+    }
+
+    // Audit Log outside the lock
+    auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+        OMAction.RECOVER_LEASE, auditMap, exception,
+        getOmRequest().getUserInfo()));
+
+    return omClientResponse;
+  }
+
+  private String doWork(OzoneManager ozoneManager,
+      RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+      throws IOException {
+
+    final long volumeId = omMetadataManager.getVolumeId(volumeName);
+    final long bucketId = omMetadataManager.getBucketId(
+        volumeName, bucketName);
+    Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+    long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+        pathComponents, keyName, omMetadataManager,
+        "Cannot recover file : " + keyName
+            + " as parent directory doesn't exist");
+    String fileName = OzoneFSUtils.getFileName(keyName);
+    String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+        parentID, fileName);
+
+    OmKeyInfo keyInfo = getKey(dbFileKey);

Review Comment:
   TestOMRecoverLeaseRequest verifies four scenarios:
   (1) recover a file that is open and hsync'ed
   (2) recover a file already closed
   (3) recover a file that is open but not hsync'ed.
   (4) recover a file that does not exist



-- 
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] jojochuang merged pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang merged PR #4234:
URL: https://github.com/apache/ozone/pull/4234


-- 
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] szetszwo commented on pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #4234:
URL: https://github.com/apache/ozone/pull/4234#issuecomment-1420228888

   @jojochuang , took a quick look.  The change looks great!
   
   We probably need to add a new `OMLayoutFeature` for hsync since we are adding new `OMClientRequest` here and added the `CommitKeyRequest.hsync` previously.


-- 
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] jojochuang commented on pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #4234:
URL: https://github.com/apache/ozone/pull/4234#issuecomment-1447172203

   > @jojochuang Also I am not able to identify use case of this API? as option from CLI is not provided.
   This is to be used by HBase and Solr as part of transaction log failure handling process.
   
   But good point on the CLI. As this is already a sizeable PR. I'll add work it as a follow-up item.


-- 
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] szetszwo commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1148492719


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,281 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private OMMetadataManager omMetadataManager;
+
+  public OMRecoverLeaseRequest(OMRequest omRequest) {
+    super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    Preconditions.checkNotNull(recoverLeaseRequest);
+    volumeName = recoverLeaseRequest.getVolumeName();
+    bucketName = recoverLeaseRequest.getBucketName();
+    keyName = recoverLeaseRequest.getKeyName();
+  }
+
+  @Override
+  public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    String keyPath = recoverLeaseRequest.getKeyName();
+    String normalizedKeyPath =
+        validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+    return getOmRequest().toBuilder()
+        .setRecoverLeaseRequest(
+            recoverLeaseRequest.toBuilder()
+                .setKeyName(normalizedKeyPath))
+        .build();
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+      long transactionLogIndex,
+      OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+    Preconditions.checkNotNull(recoverLeaseRequest);
+
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.VOLUME, volumeName);
+    auditMap.put(OzoneConsts.BUCKET, bucketName);
+    auditMap.put(OzoneConsts.KEY, keyName);
+
+    OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+        getOmRequest());
+
+    omMetadataManager = ozoneManager.getMetadataManager();
+    OMClientResponse omClientResponse = null;
+    IOException exception = null;
+    // increment metric
+    OMMetrics omMetrics = ozoneManager.getMetrics();
+
+    boolean acquiredLock = false;
+    try {
+      // check ACL
+      checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+          IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+      // acquire lock
+      acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+          volumeName, bucketName);
+
+      validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+      String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+          transactionLogIndex);
+
+      // Prepare response
+      boolean responseCode = true;
+      omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+              .setResponse(responseCode).build())
+          .setCmdType(RecoverLease);
+      omClientResponse =
+          new OMRecoverLeaseResponse(omResponse.build(), getBucketLayout(),
+              openKeyEntryName);
+      omMetrics.incNumRecoverLease();
+      LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+          bucketName, keyName);
+    } catch (IOException ex) {
+      LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+          volumeName, bucketName, keyName, ex);
+      exception = ex;
+      omMetrics.incNumRecoverLeaseFails();
+      omResponse.setCmdType(RecoverLease);
+      omClientResponse = new OMRecoverLeaseResponse(
+          createErrorOMResponse(omResponse, ex), getBucketLayout());
+    } finally {
+      if (omClientResponse != null) {
+        omClientResponse.setFlushFuture(
+            ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                transactionLogIndex));
+      }
+      if (acquiredLock) {
+        omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+            bucketName);
+      }
+    }
+
+    // Audit Log outside the lock
+    auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+        OMAction.RECOVER_LEASE, auditMap, exception,
+        getOmRequest().getUserInfo()));
+
+    return omClientResponse;
+  }
+
+  private String doWork(OzoneManager ozoneManager,
+      RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+      throws IOException {
+
+    final long volumeId = omMetadataManager.getVolumeId(volumeName);
+    final long bucketId = omMetadataManager.getBucketId(
+        volumeName, bucketName);
+    Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+    long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+        pathComponents, keyName, omMetadataManager,
+        "Cannot recover file : " + keyName
+            + " as parent directory doesn't exist");
+    String fileName = OzoneFSUtils.getFileName(keyName);
+    String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+        parentID, fileName);
+
+    OmKeyInfo keyInfo = getKey(dbFileKey);
+    if (keyInfo == null) {
+      throw new OMException("Key:" + keyName + " not found", KEY_NOT_FOUND);
+    }
+    final String clientId = keyInfo.getMetadata().get(
+        OzoneConsts.HSYNC_CLIENT_ID);
+    if (clientId == null) {
+      LOG.warn("Key:" + keyName + " is closed");
+    }
+    String openKeyEntryName = getOpenFileEntryName(volumeId, bucketId,

Review Comment:
   @jojochuang , since we have the clientId, we should use it to get `openKeyEntryName` directly instead of searching it by the prefix.
   ```java
       final String openKeyEntryName = omMetadataManager.getOpenFileName(
           volumeId, bucketId, parentID, fileName, Long.parseLong(clientId));
   ```



-- 
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] jojochuang commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1154703556


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,281 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private OMMetadataManager omMetadataManager;
+
+  public OMRecoverLeaseRequest(OMRequest omRequest) {
+    super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    Preconditions.checkNotNull(recoverLeaseRequest);
+    volumeName = recoverLeaseRequest.getVolumeName();
+    bucketName = recoverLeaseRequest.getBucketName();
+    keyName = recoverLeaseRequest.getKeyName();
+  }
+
+  @Override
+  public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    String keyPath = recoverLeaseRequest.getKeyName();
+    String normalizedKeyPath =
+        validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+    return getOmRequest().toBuilder()
+        .setRecoverLeaseRequest(
+            recoverLeaseRequest.toBuilder()
+                .setKeyName(normalizedKeyPath))
+        .build();
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+      long transactionLogIndex,
+      OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+    Preconditions.checkNotNull(recoverLeaseRequest);
+
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.VOLUME, volumeName);
+    auditMap.put(OzoneConsts.BUCKET, bucketName);
+    auditMap.put(OzoneConsts.KEY, keyName);
+
+    OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+        getOmRequest());
+
+    omMetadataManager = ozoneManager.getMetadataManager();
+    OMClientResponse omClientResponse = null;
+    IOException exception = null;
+    // increment metric
+    OMMetrics omMetrics = ozoneManager.getMetrics();
+
+    boolean acquiredLock = false;
+    try {
+      // check ACL
+      checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+          IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+      // acquire lock
+      acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+          volumeName, bucketName);
+
+      validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+      String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+          transactionLogIndex);
+
+      // Prepare response
+      boolean responseCode = true;
+      omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+              .setResponse(responseCode).build())
+          .setCmdType(RecoverLease);
+      omClientResponse =
+          new OMRecoverLeaseResponse(omResponse.build(), getBucketLayout(),
+              openKeyEntryName);
+      omMetrics.incNumRecoverLease();
+      LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+          bucketName, keyName);
+    } catch (IOException ex) {
+      LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+          volumeName, bucketName, keyName, ex);
+      exception = ex;
+      omMetrics.incNumRecoverLeaseFails();
+      omResponse.setCmdType(RecoverLease);
+      omClientResponse = new OMRecoverLeaseResponse(
+          createErrorOMResponse(omResponse, ex), getBucketLayout());
+    } finally {
+      if (omClientResponse != null) {
+        omClientResponse.setFlushFuture(
+            ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                transactionLogIndex));
+      }
+      if (acquiredLock) {
+        omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+            bucketName);
+      }
+    }
+
+    // Audit Log outside the lock
+    auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+        OMAction.RECOVER_LEASE, auditMap, exception,
+        getOmRequest().getUserInfo()));
+
+    return omClientResponse;
+  }
+
+  private String doWork(OzoneManager ozoneManager,
+      RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+      throws IOException {
+
+    final long volumeId = omMetadataManager.getVolumeId(volumeName);
+    final long bucketId = omMetadataManager.getBucketId(
+        volumeName, bucketName);
+    Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+    long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+        pathComponents, keyName, omMetadataManager,
+        "Cannot recover file : " + keyName
+            + " as parent directory doesn't exist");
+    String fileName = OzoneFSUtils.getFileName(keyName);
+    String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+        parentID, fileName);
+
+    OmKeyInfo keyInfo = getKey(dbFileKey);
+    if (keyInfo == null) {
+      throw new OMException("Key:" + keyName + " not found", KEY_NOT_FOUND);
+    }
+    final String clientId = keyInfo.getMetadata().get(
+        OzoneConsts.HSYNC_CLIENT_ID);
+    if (clientId == null) {
+      LOG.warn("Key:" + keyName + " is closed");
+    }
+    String openKeyEntryName = getOpenFileEntryName(volumeId, bucketId,
+        parentID, fileName);
+    if (openKeyEntryName != null) {
+      checkFileState(keyInfo, openKeyEntryName);
+      commitKey(dbFileKey, keyInfo, ozoneManager, transactionLogIndex);
+      removeOpenKey(openKeyEntryName, transactionLogIndex);
+    }
+
+    return openKeyEntryName;
+  }
+
+  private OmKeyInfo getKey(String dbOzoneKey) throws IOException {
+    return omMetadataManager.getKeyTable(getBucketLayout()).get(dbOzoneKey);
+  }
+
+  private String getOpenFileEntryName(long volumeId, long bucketId,
+      long parentObjectId, String fileName) throws IOException {
+    String openFileEntryName = null;
+    String dbOpenKeyPrefix =
+        omMetadataManager.getOpenFileNamePrefix(volumeId, bucketId,
+            parentObjectId, fileName);
+    try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
+        iter = omMetadataManager.getOpenKeyTable(getBucketLayout())
+        .iterator(dbOpenKeyPrefix)) {
+
+      while (iter.hasNext()) {
+        if (openFileEntryName != null) {
+          throw new IOException("Found more than two keys in the" +
+            " open key table for file" + fileName);
+        }
+        openFileEntryName = iter.next().getKey();
+      }
+    }
+    /*if (openFileEntryName == null) {
+      throw new IOException("Unable to find the corresponding key in " +
+        "the open key table for file " + fileName);
+    }*/
+    return openFileEntryName;
+  }
+
+  private void checkFileState(OmKeyInfo keyInfo, String openKeyEntryName)
+      throws IOException {
+
+    // if file is closed, do nothing and return right away.
+    if (openKeyEntryName.isEmpty()) {
+      return; // ("File is closed");
+    }
+    // if file is open, no sync, fail right away.
+    if (keyInfo == null) {
+      throw new IOException("file is open but not yet sync'ed");
+    }
+  }
+
+  private void commitKey(String dbOzoneKey, OmKeyInfo omKeyInfo,
+      OzoneManager ozoneManager, long transactionLogIndex) throws IOException {
+    omKeyInfo.setModificationTime(Time.now());
+    omKeyInfo.setUpdateID(transactionLogIndex, ozoneManager.isRatisEnabled());

Review Comment:
   IMO i'm inclined to not support lease recovery and hsync for MPK. Hsync and lease recovery is a behavior tailored for HDFS applications and MPK was not support in HDFS. But let me check if we need to add guardrails or tests to verify.



-- 
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] jojochuang commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1154703556


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,281 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private OMMetadataManager omMetadataManager;
+
+  public OMRecoverLeaseRequest(OMRequest omRequest) {
+    super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    Preconditions.checkNotNull(recoverLeaseRequest);
+    volumeName = recoverLeaseRequest.getVolumeName();
+    bucketName = recoverLeaseRequest.getBucketName();
+    keyName = recoverLeaseRequest.getKeyName();
+  }
+
+  @Override
+  public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    String keyPath = recoverLeaseRequest.getKeyName();
+    String normalizedKeyPath =
+        validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+    return getOmRequest().toBuilder()
+        .setRecoverLeaseRequest(
+            recoverLeaseRequest.toBuilder()
+                .setKeyName(normalizedKeyPath))
+        .build();
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+      long transactionLogIndex,
+      OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+    Preconditions.checkNotNull(recoverLeaseRequest);
+
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.VOLUME, volumeName);
+    auditMap.put(OzoneConsts.BUCKET, bucketName);
+    auditMap.put(OzoneConsts.KEY, keyName);
+
+    OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+        getOmRequest());
+
+    omMetadataManager = ozoneManager.getMetadataManager();
+    OMClientResponse omClientResponse = null;
+    IOException exception = null;
+    // increment metric
+    OMMetrics omMetrics = ozoneManager.getMetrics();
+
+    boolean acquiredLock = false;
+    try {
+      // check ACL
+      checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+          IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+      // acquire lock
+      acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+          volumeName, bucketName);
+
+      validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+      String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+          transactionLogIndex);
+
+      // Prepare response
+      boolean responseCode = true;
+      omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+              .setResponse(responseCode).build())
+          .setCmdType(RecoverLease);
+      omClientResponse =
+          new OMRecoverLeaseResponse(omResponse.build(), getBucketLayout(),
+              openKeyEntryName);
+      omMetrics.incNumRecoverLease();
+      LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+          bucketName, keyName);
+    } catch (IOException ex) {
+      LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+          volumeName, bucketName, keyName, ex);
+      exception = ex;
+      omMetrics.incNumRecoverLeaseFails();
+      omResponse.setCmdType(RecoverLease);
+      omClientResponse = new OMRecoverLeaseResponse(
+          createErrorOMResponse(omResponse, ex), getBucketLayout());
+    } finally {
+      if (omClientResponse != null) {
+        omClientResponse.setFlushFuture(
+            ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                transactionLogIndex));
+      }
+      if (acquiredLock) {
+        omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+            bucketName);
+      }
+    }
+
+    // Audit Log outside the lock
+    auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+        OMAction.RECOVER_LEASE, auditMap, exception,
+        getOmRequest().getUserInfo()));
+
+    return omClientResponse;
+  }
+
+  private String doWork(OzoneManager ozoneManager,
+      RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+      throws IOException {
+
+    final long volumeId = omMetadataManager.getVolumeId(volumeName);
+    final long bucketId = omMetadataManager.getBucketId(
+        volumeName, bucketName);
+    Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+    long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+        pathComponents, keyName, omMetadataManager,
+        "Cannot recover file : " + keyName
+            + " as parent directory doesn't exist");
+    String fileName = OzoneFSUtils.getFileName(keyName);
+    String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+        parentID, fileName);
+
+    OmKeyInfo keyInfo = getKey(dbFileKey);
+    if (keyInfo == null) {
+      throw new OMException("Key:" + keyName + " not found", KEY_NOT_FOUND);
+    }
+    final String clientId = keyInfo.getMetadata().get(
+        OzoneConsts.HSYNC_CLIENT_ID);
+    if (clientId == null) {
+      LOG.warn("Key:" + keyName + " is closed");
+    }
+    String openKeyEntryName = getOpenFileEntryName(volumeId, bucketId,
+        parentID, fileName);
+    if (openKeyEntryName != null) {
+      checkFileState(keyInfo, openKeyEntryName);
+      commitKey(dbFileKey, keyInfo, ozoneManager, transactionLogIndex);
+      removeOpenKey(openKeyEntryName, transactionLogIndex);
+    }
+
+    return openKeyEntryName;
+  }
+
+  private OmKeyInfo getKey(String dbOzoneKey) throws IOException {
+    return omMetadataManager.getKeyTable(getBucketLayout()).get(dbOzoneKey);
+  }
+
+  private String getOpenFileEntryName(long volumeId, long bucketId,
+      long parentObjectId, String fileName) throws IOException {
+    String openFileEntryName = null;
+    String dbOpenKeyPrefix =
+        omMetadataManager.getOpenFileNamePrefix(volumeId, bucketId,
+            parentObjectId, fileName);
+    try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
+        iter = omMetadataManager.getOpenKeyTable(getBucketLayout())
+        .iterator(dbOpenKeyPrefix)) {
+
+      while (iter.hasNext()) {
+        if (openFileEntryName != null) {
+          throw new IOException("Found more than two keys in the" +
+            " open key table for file" + fileName);
+        }
+        openFileEntryName = iter.next().getKey();
+      }
+    }
+    /*if (openFileEntryName == null) {
+      throw new IOException("Unable to find the corresponding key in " +
+        "the open key table for file " + fileName);
+    }*/
+    return openFileEntryName;
+  }
+
+  private void checkFileState(OmKeyInfo keyInfo, String openKeyEntryName)
+      throws IOException {
+
+    // if file is closed, do nothing and return right away.
+    if (openKeyEntryName.isEmpty()) {
+      return; // ("File is closed");
+    }
+    // if file is open, no sync, fail right away.
+    if (keyInfo == null) {
+      throw new IOException("file is open but not yet sync'ed");
+    }
+  }
+
+  private void commitKey(String dbOzoneKey, OmKeyInfo omKeyInfo,
+      OzoneManager ozoneManager, long transactionLogIndex) throws IOException {
+    omKeyInfo.setModificationTime(Time.now());
+    omKeyInfo.setUpdateID(transactionLogIndex, ozoneManager.isRatisEnabled());

Review Comment:
   IMO i'm inclined to not support lease recovery and hsync for MPK. Hsync and lease recovery is a behavior tailor for HDFS applications and MPK was not support in HDFS. But let me check if we need to add guardrails or tests to verify.



-- 
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 pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "umamaheswararao (via GitHub)" <gi...@apache.org>.
umamaheswararao commented on PR #4234:
URL: https://github.com/apache/ozone/pull/4234#issuecomment-1419431671

   @sumitagrawl 


-- 
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] jojochuang commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1119356844


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,264 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+    private static final Logger LOG =
+            LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+    RecoverLeaseRequest recoverLeaseRequest;
+
+    private String volumeName;
+    private String bucketName;
+    private String keyName;
+
+    private String normalizedKeyPath;
+
+    private OMMetadataManager omMetadataManager;
+
+    public OMRecoverLeaseRequest(OMRequest omRequest) {
+        super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+        RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+            .getRecoverLeaseRequest();
+        Preconditions.checkNotNull(recoverLeaseRequest);
+        volumeName = recoverLeaseRequest.getVolumeName();
+        bucketName = recoverLeaseRequest.getBucketName();
+        keyName = recoverLeaseRequest.getKeyName();
+    }
+
+    @Override
+    public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+        RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+                .getRecoverLeaseRequest();
+
+        String keyPath = recoverLeaseRequest.getKeyName();
+        normalizedKeyPath = validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+        return getOmRequest().toBuilder()
+            .setRecoverLeaseRequest(
+                recoverLeaseRequest.toBuilder()
+                    .setKeyName(normalizedKeyPath))
+            .build();
+    }
+
+    @Override
+    public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, long transactionLogIndex, OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+        RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+                .getRecoverLeaseRequest();
+        Preconditions.checkNotNull(recoverLeaseRequest);
+
+        Map<String, String> auditMap = new LinkedHashMap<>();
+        auditMap.put(OzoneConsts.VOLUME, volumeName);
+        auditMap.put(OzoneConsts.BUCKET, bucketName);
+        auditMap.put(OzoneConsts.KEY, keyName);
+
+        OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+                getOmRequest());
+
+        omMetadataManager = ozoneManager.getMetadataManager();
+        OMClientResponse omClientResponse = null;
+        IOException exception = null;
+        // increment metric
+        OMMetrics omMetrics = ozoneManager.getMetrics();
+
+        boolean acquiredLock = false;
+        try {
+            // check ACL
+            checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+                    IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+            // acquire lock
+            acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+                    volumeName, bucketName);
+
+            validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+            String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+                transactionLogIndex);
+
+            // Prepare response
+            boolean responseCode = true;
+            omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+                            .setResponse(responseCode).build())
+                    .setCmdType(RecoverLease);
+            omClientResponse = new OMRecoverLeaseResponse(omResponse.build(),
+                    getBucketLayout(), openKeyEntryName);
+            omMetrics.incNumRecoverLease();
+            LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+                bucketName, keyName);
+        } catch (IOException ex) {
+            LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+                volumeName, bucketName, keyName, ex);
+            exception = ex;
+            omMetrics.incNumRecoverLeaseFails();
+            omResponse.setCmdType(RecoverLease);
+            omClientResponse = new OMRecoverLeaseResponse(
+                createErrorOMResponse(omResponse, ex), getBucketLayout(), null);
+        } finally {
+            if (omClientResponse != null) {
+                omClientResponse.setFlushFuture(
+                        ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                                transactionLogIndex));
+            }
+            if (acquiredLock) {
+                omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+                        bucketName);
+            }
+        }
+
+        // Audit Log outside the lock
+        auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+                OMAction.RECOVER_LEASE, auditMap, exception,
+                getOmRequest().getUserInfo()));
+
+        return omClientResponse;
+    }
+
+    private String doWork(OzoneManager ozoneManager,
+        RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+        throws IOException {
+
+        //String dbOzoneKey =
+        //    omMetadataManager.getOzoneKey(volumeName, bucketName,
+        //        keyName);
+        final long volumeId = omMetadataManager.getVolumeId(volumeName);
+        final long bucketId = omMetadataManager.getBucketId(
+            volumeName, bucketName);
+        Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+        long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+            pathComponents, keyName, omMetadataManager,
+            "Cannot recover file : " + keyName
+                + " as parent directory doesn't exist");
+        String fileName = OzoneFSUtils.getFileName(keyName);
+        String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+            parentID, fileName);
+
+        OmKeyInfo keyInfo = getKey(dbFileKey);
+        String openKeyEntryName = getOpenFileEntryName(volumeId, bucketId, parentID, fileName);
+
+        checkFileState(keyInfo, openKeyEntryName);
+
+        commitKey(dbFileKey, keyInfo, ozoneManager, transactionLogIndex);
+
+        removeOpenKey(openKeyEntryName, transactionLogIndex);
+
+        return openKeyEntryName;
+    }
+
+    private OmKeyInfo getKey(String dbOzoneKey) throws IOException {
+        return omMetadataManager.getKeyTable(getBucketLayout()).get(dbOzoneKey);
+    }
+
+    private String getOpenFileEntryName(long volumeId, long bucketId,
+        long parentObjectId, String fileName) throws IOException {
+        String openFileEntryName = "";
+        String dbOpenKeyPrefix =
+            omMetadataManager.getOpenFileNamePrefix(volumeId, bucketId, parentObjectId, fileName);
+        try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
+            iter = omMetadataManager.getOpenKeyTable(getBucketLayout())
+            .iterator(dbOpenKeyPrefix)) {
+
+            // TODO: verify that there should only be one entry for the open key

Review Comment:
   This would need to be similar to HDFS



-- 
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] jojochuang commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1148483402


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,278 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private String normalizedKeyPath;
+
+  private OMMetadataManager omMetadataManager;
+
+  public OMRecoverLeaseRequest(OMRequest omRequest) {
+    super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    Preconditions.checkNotNull(recoverLeaseRequest);
+    volumeName = recoverLeaseRequest.getVolumeName();
+    bucketName = recoverLeaseRequest.getBucketName();
+    keyName = recoverLeaseRequest.getKeyName();
+  }
+
+  @Override
+  public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    String keyPath = recoverLeaseRequest.getKeyName();
+    normalizedKeyPath =
+        validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+        keyPath, getBucketLayout());
+
+    return getOmRequest().toBuilder()
+        .setRecoverLeaseRequest(
+            recoverLeaseRequest.toBuilder()
+                .setKeyName(normalizedKeyPath))
+        .build();
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+      long transactionLogIndex,
+      OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+    Preconditions.checkNotNull(recoverLeaseRequest);
+
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.VOLUME, volumeName);
+    auditMap.put(OzoneConsts.BUCKET, bucketName);
+    auditMap.put(OzoneConsts.KEY, keyName);
+
+    OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+        getOmRequest());
+
+    omMetadataManager = ozoneManager.getMetadataManager();
+    OMClientResponse omClientResponse = null;
+    IOException exception = null;
+    // increment metric
+    OMMetrics omMetrics = ozoneManager.getMetrics();
+
+    boolean acquiredLock = false;
+    try {
+      // check ACL
+      checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+          IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+      // acquire lock
+      acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+          volumeName, bucketName);
+
+      validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+      String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+          transactionLogIndex);
+
+      // Prepare response
+      boolean responseCode = true;
+      omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+              .setResponse(responseCode).build())
+          .setCmdType(RecoverLease);
+      omClientResponse =
+          new OMRecoverLeaseResponse(omResponse.build(), getBucketLayout(),
+              openKeyEntryName);
+      omMetrics.incNumRecoverLease();
+      LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+          bucketName, keyName);
+    } catch (IOException ex) {
+      LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+          volumeName, bucketName, keyName, ex);
+      exception = ex;
+      omMetrics.incNumRecoverLeaseFails();
+      omResponse.setCmdType(RecoverLease);
+      omClientResponse = new OMRecoverLeaseResponse(
+          createErrorOMResponse(omResponse, ex), getBucketLayout());
+    } finally {
+      if (omClientResponse != null) {
+        omClientResponse.setFlushFuture(
+            ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                transactionLogIndex));
+      }
+      if (acquiredLock) {
+        omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+            bucketName);
+      }
+    }
+
+    // Audit Log outside the lock
+    auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+        OMAction.RECOVER_LEASE, auditMap, exception,
+        getOmRequest().getUserInfo()));
+
+    return omClientResponse;
+  }
+
+  private String doWork(OzoneManager ozoneManager,
+      RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+      throws IOException {
+
+    final long volumeId = omMetadataManager.getVolumeId(volumeName);
+    final long bucketId = omMetadataManager.getBucketId(
+        volumeName, bucketName);
+    Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+    long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+        pathComponents, keyName, omMetadataManager,
+        "Cannot recover file : " + keyName
+            + " as parent directory doesn't exist");
+    String fileName = OzoneFSUtils.getFileName(keyName);
+    String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+        parentID, fileName);
+
+    OmKeyInfo keyInfo = getKey(dbFileKey);
+    if (keyInfo == null) {
+      throw new OMException("Key:" + keyName + " not found", KEY_NOT_FOUND);
+    }
+    String openKeyEntryName = getOpenFileEntryName(volumeId, bucketId,
+        parentID, fileName);
+    if (openKeyEntryName != null) {
+      checkFileState(keyInfo, openKeyEntryName);
+      commitKey(dbFileKey, keyInfo, ozoneManager, transactionLogIndex);
+      removeOpenKey(openKeyEntryName, transactionLogIndex);
+    }

Review Comment:
   Actually now that I run this through TestOMRecoverLeaseRequest, I realized that this is not the same as HDFS. In HDFS, closing a closed file is allowed, essentially a no-op. HBase WAL writer actually calls recover lease before it starts, even if the WAL file is closed. So adding this check would break HBase.
   
   What if we just log a warning instead?



-- 
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] szetszwo commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1148494115


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,281 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private OMMetadataManager omMetadataManager;
+
+  public OMRecoverLeaseRequest(OMRequest omRequest) {
+    super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    Preconditions.checkNotNull(recoverLeaseRequest);
+    volumeName = recoverLeaseRequest.getVolumeName();
+    bucketName = recoverLeaseRequest.getBucketName();
+    keyName = recoverLeaseRequest.getKeyName();
+  }
+
+  @Override
+  public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    String keyPath = recoverLeaseRequest.getKeyName();
+    String normalizedKeyPath =
+        validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+    return getOmRequest().toBuilder()
+        .setRecoverLeaseRequest(
+            recoverLeaseRequest.toBuilder()
+                .setKeyName(normalizedKeyPath))
+        .build();
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+      long transactionLogIndex,
+      OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+    Preconditions.checkNotNull(recoverLeaseRequest);
+
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.VOLUME, volumeName);
+    auditMap.put(OzoneConsts.BUCKET, bucketName);
+    auditMap.put(OzoneConsts.KEY, keyName);
+
+    OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+        getOmRequest());
+
+    omMetadataManager = ozoneManager.getMetadataManager();
+    OMClientResponse omClientResponse = null;
+    IOException exception = null;
+    // increment metric
+    OMMetrics omMetrics = ozoneManager.getMetrics();
+
+    boolean acquiredLock = false;
+    try {
+      // check ACL
+      checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+          IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+      // acquire lock
+      acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+          volumeName, bucketName);
+
+      validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+      String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+          transactionLogIndex);
+
+      // Prepare response
+      boolean responseCode = true;
+      omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+              .setResponse(responseCode).build())
+          .setCmdType(RecoverLease);
+      omClientResponse =
+          new OMRecoverLeaseResponse(omResponse.build(), getBucketLayout(),
+              openKeyEntryName);
+      omMetrics.incNumRecoverLease();
+      LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+          bucketName, keyName);
+    } catch (IOException ex) {
+      LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+          volumeName, bucketName, keyName, ex);
+      exception = ex;
+      omMetrics.incNumRecoverLeaseFails();
+      omResponse.setCmdType(RecoverLease);
+      omClientResponse = new OMRecoverLeaseResponse(
+          createErrorOMResponse(omResponse, ex), getBucketLayout());
+    } finally {
+      if (omClientResponse != null) {
+        omClientResponse.setFlushFuture(
+            ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                transactionLogIndex));
+      }
+      if (acquiredLock) {
+        omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+            bucketName);
+      }
+    }
+
+    // Audit Log outside the lock
+    auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+        OMAction.RECOVER_LEASE, auditMap, exception,
+        getOmRequest().getUserInfo()));
+
+    return omClientResponse;
+  }
+
+  private String doWork(OzoneManager ozoneManager,
+      RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+      throws IOException {
+
+    final long volumeId = omMetadataManager.getVolumeId(volumeName);
+    final long bucketId = omMetadataManager.getBucketId(
+        volumeName, bucketName);
+    Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+    long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+        pathComponents, keyName, omMetadataManager,
+        "Cannot recover file : " + keyName
+            + " as parent directory doesn't exist");
+    String fileName = OzoneFSUtils.getFileName(keyName);
+    String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+        parentID, fileName);
+
+    OmKeyInfo keyInfo = getKey(dbFileKey);
+    if (keyInfo == null) {
+      throw new OMException("Key:" + keyName + " not found", KEY_NOT_FOUND);
+    }
+    final String clientId = keyInfo.getMetadata().get(
+        OzoneConsts.HSYNC_CLIENT_ID);
+    if (clientId == null) {
+      LOG.warn("Key:" + keyName + " is closed");

Review Comment:
   When `clientId == null`, should it abort recover lease and return immediately?



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,281 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private OMMetadataManager omMetadataManager;
+
+  public OMRecoverLeaseRequest(OMRequest omRequest) {
+    super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    Preconditions.checkNotNull(recoverLeaseRequest);
+    volumeName = recoverLeaseRequest.getVolumeName();
+    bucketName = recoverLeaseRequest.getBucketName();
+    keyName = recoverLeaseRequest.getKeyName();
+  }
+
+  @Override
+  public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    String keyPath = recoverLeaseRequest.getKeyName();
+    String normalizedKeyPath =
+        validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+    return getOmRequest().toBuilder()
+        .setRecoverLeaseRequest(
+            recoverLeaseRequest.toBuilder()
+                .setKeyName(normalizedKeyPath))
+        .build();
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+      long transactionLogIndex,
+      OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+    Preconditions.checkNotNull(recoverLeaseRequest);
+
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.VOLUME, volumeName);
+    auditMap.put(OzoneConsts.BUCKET, bucketName);
+    auditMap.put(OzoneConsts.KEY, keyName);
+
+    OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+        getOmRequest());
+
+    omMetadataManager = ozoneManager.getMetadataManager();
+    OMClientResponse omClientResponse = null;
+    IOException exception = null;
+    // increment metric
+    OMMetrics omMetrics = ozoneManager.getMetrics();
+
+    boolean acquiredLock = false;
+    try {
+      // check ACL
+      checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+          IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+      // acquire lock
+      acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+          volumeName, bucketName);
+
+      validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+      String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+          transactionLogIndex);
+
+      // Prepare response
+      boolean responseCode = true;
+      omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+              .setResponse(responseCode).build())
+          .setCmdType(RecoverLease);
+      omClientResponse =
+          new OMRecoverLeaseResponse(omResponse.build(), getBucketLayout(),
+              openKeyEntryName);
+      omMetrics.incNumRecoverLease();
+      LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+          bucketName, keyName);
+    } catch (IOException ex) {
+      LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+          volumeName, bucketName, keyName, ex);
+      exception = ex;
+      omMetrics.incNumRecoverLeaseFails();
+      omResponse.setCmdType(RecoverLease);
+      omClientResponse = new OMRecoverLeaseResponse(
+          createErrorOMResponse(omResponse, ex), getBucketLayout());
+    } finally {
+      if (omClientResponse != null) {
+        omClientResponse.setFlushFuture(
+            ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                transactionLogIndex));
+      }
+      if (acquiredLock) {
+        omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+            bucketName);
+      }
+    }
+
+    // Audit Log outside the lock
+    auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+        OMAction.RECOVER_LEASE, auditMap, exception,
+        getOmRequest().getUserInfo()));
+
+    return omClientResponse;
+  }
+
+  private String doWork(OzoneManager ozoneManager,
+      RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+      throws IOException {
+
+    final long volumeId = omMetadataManager.getVolumeId(volumeName);
+    final long bucketId = omMetadataManager.getBucketId(
+        volumeName, bucketName);
+    Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+    long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+        pathComponents, keyName, omMetadataManager,
+        "Cannot recover file : " + keyName
+            + " as parent directory doesn't exist");
+    String fileName = OzoneFSUtils.getFileName(keyName);
+    String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+        parentID, fileName);
+
+    OmKeyInfo keyInfo = getKey(dbFileKey);
+    if (keyInfo == null) {
+      throw new OMException("Key:" + keyName + " not found", KEY_NOT_FOUND);
+    }
+    final String clientId = keyInfo.getMetadata().get(
+        OzoneConsts.HSYNC_CLIENT_ID);
+    if (clientId == null) {
+      LOG.warn("Key:" + keyName + " is closed");
+    }
+    String openKeyEntryName = getOpenFileEntryName(volumeId, bucketId,
+        parentID, fileName);
+    if (openKeyEntryName != null) {
+      checkFileState(keyInfo, openKeyEntryName);
+      commitKey(dbFileKey, keyInfo, ozoneManager, transactionLogIndex);
+      removeOpenKey(openKeyEntryName, transactionLogIndex);
+    }
+
+    return openKeyEntryName;
+  }
+
+  private OmKeyInfo getKey(String dbOzoneKey) throws IOException {
+    return omMetadataManager.getKeyTable(getBucketLayout()).get(dbOzoneKey);
+  }
+
+  private String getOpenFileEntryName(long volumeId, long bucketId,
+      long parentObjectId, String fileName) throws IOException {
+    String openFileEntryName = null;
+    String dbOpenKeyPrefix =
+        omMetadataManager.getOpenFileNamePrefix(volumeId, bucketId,
+            parentObjectId, fileName);
+    try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
+        iter = omMetadataManager.getOpenKeyTable(getBucketLayout())
+        .iterator(dbOpenKeyPrefix)) {
+
+      while (iter.hasNext()) {
+        if (openFileEntryName != null) {
+          throw new IOException("Found more than two keys in the" +
+            " open key table for file" + fileName);
+        }
+        openFileEntryName = iter.next().getKey();
+      }
+    }
+    /*if (openFileEntryName == null) {
+      throw new IOException("Unable to find the corresponding key in " +
+        "the open key table for file " + fileName);
+    }*/
+    return openFileEntryName;
+  }
+
+  private void checkFileState(OmKeyInfo keyInfo, String openKeyEntryName)
+      throws IOException {
+
+    // if file is closed, do nothing and return right away.
+    if (openKeyEntryName.isEmpty()) {
+      return; // ("File is closed");
+    }
+    // if file is open, no sync, fail right away.
+    if (keyInfo == null) {
+      throw new IOException("file is open but not yet sync'ed");
+    }
+  }
+
+  private void commitKey(String dbOzoneKey, OmKeyInfo omKeyInfo,
+      OzoneManager ozoneManager, long transactionLogIndex) throws IOException {
+    omKeyInfo.setModificationTime(Time.now());
+    omKeyInfo.setUpdateID(transactionLogIndex, ozoneManager.isRatisEnabled());
+
+    omMetadataManager.getKeyTable(getBucketLayout()).addCacheEntry(
+      new CacheKey<>(dbOzoneKey),
+      new CacheValue<>(Optional.of(omKeyInfo), transactionLogIndex));

Review Comment:
   Use `CacheValue.get(transactionLogIndex, omKeyInfo)`.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,281 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private OMMetadataManager omMetadataManager;
+
+  public OMRecoverLeaseRequest(OMRequest omRequest) {
+    super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    Preconditions.checkNotNull(recoverLeaseRequest);
+    volumeName = recoverLeaseRequest.getVolumeName();
+    bucketName = recoverLeaseRequest.getBucketName();
+    keyName = recoverLeaseRequest.getKeyName();
+  }
+
+  @Override
+  public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    String keyPath = recoverLeaseRequest.getKeyName();
+    String normalizedKeyPath =
+        validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+    return getOmRequest().toBuilder()
+        .setRecoverLeaseRequest(
+            recoverLeaseRequest.toBuilder()
+                .setKeyName(normalizedKeyPath))
+        .build();
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+      long transactionLogIndex,
+      OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+    Preconditions.checkNotNull(recoverLeaseRequest);
+
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.VOLUME, volumeName);
+    auditMap.put(OzoneConsts.BUCKET, bucketName);
+    auditMap.put(OzoneConsts.KEY, keyName);
+
+    OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+        getOmRequest());
+
+    omMetadataManager = ozoneManager.getMetadataManager();
+    OMClientResponse omClientResponse = null;
+    IOException exception = null;
+    // increment metric
+    OMMetrics omMetrics = ozoneManager.getMetrics();
+
+    boolean acquiredLock = false;
+    try {
+      // check ACL
+      checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+          IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+      // acquire lock
+      acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+          volumeName, bucketName);
+
+      validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+      String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+          transactionLogIndex);
+
+      // Prepare response
+      boolean responseCode = true;
+      omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+              .setResponse(responseCode).build())
+          .setCmdType(RecoverLease);
+      omClientResponse =
+          new OMRecoverLeaseResponse(omResponse.build(), getBucketLayout(),
+              openKeyEntryName);
+      omMetrics.incNumRecoverLease();
+      LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+          bucketName, keyName);
+    } catch (IOException ex) {
+      LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+          volumeName, bucketName, keyName, ex);
+      exception = ex;
+      omMetrics.incNumRecoverLeaseFails();
+      omResponse.setCmdType(RecoverLease);
+      omClientResponse = new OMRecoverLeaseResponse(
+          createErrorOMResponse(omResponse, ex), getBucketLayout());
+    } finally {
+      if (omClientResponse != null) {
+        omClientResponse.setFlushFuture(
+            ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                transactionLogIndex));
+      }
+      if (acquiredLock) {
+        omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+            bucketName);
+      }
+    }
+
+    // Audit Log outside the lock
+    auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+        OMAction.RECOVER_LEASE, auditMap, exception,
+        getOmRequest().getUserInfo()));
+
+    return omClientResponse;
+  }
+
+  private String doWork(OzoneManager ozoneManager,
+      RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+      throws IOException {
+
+    final long volumeId = omMetadataManager.getVolumeId(volumeName);
+    final long bucketId = omMetadataManager.getBucketId(
+        volumeName, bucketName);
+    Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+    long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+        pathComponents, keyName, omMetadataManager,
+        "Cannot recover file : " + keyName
+            + " as parent directory doesn't exist");
+    String fileName = OzoneFSUtils.getFileName(keyName);
+    String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+        parentID, fileName);
+
+    OmKeyInfo keyInfo = getKey(dbFileKey);
+    if (keyInfo == null) {
+      throw new OMException("Key:" + keyName + " not found", KEY_NOT_FOUND);
+    }
+    final String clientId = keyInfo.getMetadata().get(
+        OzoneConsts.HSYNC_CLIENT_ID);
+    if (clientId == null) {
+      LOG.warn("Key:" + keyName + " is closed");
+    }
+    String openKeyEntryName = getOpenFileEntryName(volumeId, bucketId,
+        parentID, fileName);
+    if (openKeyEntryName != null) {
+      checkFileState(keyInfo, openKeyEntryName);
+      commitKey(dbFileKey, keyInfo, ozoneManager, transactionLogIndex);
+      removeOpenKey(openKeyEntryName, transactionLogIndex);
+    }
+
+    return openKeyEntryName;
+  }
+
+  private OmKeyInfo getKey(String dbOzoneKey) throws IOException {
+    return omMetadataManager.getKeyTable(getBucketLayout()).get(dbOzoneKey);
+  }
+
+  private String getOpenFileEntryName(long volumeId, long bucketId,
+      long parentObjectId, String fileName) throws IOException {
+    String openFileEntryName = null;
+    String dbOpenKeyPrefix =
+        omMetadataManager.getOpenFileNamePrefix(volumeId, bucketId,
+            parentObjectId, fileName);
+    try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
+        iter = omMetadataManager.getOpenKeyTable(getBucketLayout())
+        .iterator(dbOpenKeyPrefix)) {
+
+      while (iter.hasNext()) {
+        if (openFileEntryName != null) {
+          throw new IOException("Found more than two keys in the" +
+            " open key table for file" + fileName);
+        }
+        openFileEntryName = iter.next().getKey();
+      }
+    }
+    /*if (openFileEntryName == null) {
+      throw new IOException("Unable to find the corresponding key in " +
+        "the open key table for file " + fileName);
+    }*/
+    return openFileEntryName;
+  }
+
+  private void checkFileState(OmKeyInfo keyInfo, String openKeyEntryName)
+      throws IOException {
+
+    // if file is closed, do nothing and return right away.
+    if (openKeyEntryName.isEmpty()) {
+      return; // ("File is closed");
+    }
+    // if file is open, no sync, fail right away.
+    if (keyInfo == null) {
+      throw new IOException("file is open but not yet sync'ed");
+    }
+  }
+
+  private void commitKey(String dbOzoneKey, OmKeyInfo omKeyInfo,
+      OzoneManager ozoneManager, long transactionLogIndex) throws IOException {

Review Comment:
   Remove `throws IOException`.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,281 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private OMMetadataManager omMetadataManager;
+
+  public OMRecoverLeaseRequest(OMRequest omRequest) {
+    super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    Preconditions.checkNotNull(recoverLeaseRequest);
+    volumeName = recoverLeaseRequest.getVolumeName();
+    bucketName = recoverLeaseRequest.getBucketName();
+    keyName = recoverLeaseRequest.getKeyName();
+  }
+
+  @Override
+  public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    String keyPath = recoverLeaseRequest.getKeyName();
+    String normalizedKeyPath =
+        validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+    return getOmRequest().toBuilder()
+        .setRecoverLeaseRequest(
+            recoverLeaseRequest.toBuilder()
+                .setKeyName(normalizedKeyPath))
+        .build();
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+      long transactionLogIndex,
+      OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+    Preconditions.checkNotNull(recoverLeaseRequest);
+
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.VOLUME, volumeName);
+    auditMap.put(OzoneConsts.BUCKET, bucketName);
+    auditMap.put(OzoneConsts.KEY, keyName);
+
+    OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+        getOmRequest());
+
+    omMetadataManager = ozoneManager.getMetadataManager();
+    OMClientResponse omClientResponse = null;
+    IOException exception = null;
+    // increment metric
+    OMMetrics omMetrics = ozoneManager.getMetrics();
+
+    boolean acquiredLock = false;
+    try {
+      // check ACL
+      checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+          IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+      // acquire lock
+      acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+          volumeName, bucketName);
+
+      validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+      String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+          transactionLogIndex);
+
+      // Prepare response
+      boolean responseCode = true;
+      omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+              .setResponse(responseCode).build())
+          .setCmdType(RecoverLease);
+      omClientResponse =
+          new OMRecoverLeaseResponse(omResponse.build(), getBucketLayout(),
+              openKeyEntryName);
+      omMetrics.incNumRecoverLease();
+      LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+          bucketName, keyName);
+    } catch (IOException ex) {
+      LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+          volumeName, bucketName, keyName, ex);
+      exception = ex;
+      omMetrics.incNumRecoverLeaseFails();
+      omResponse.setCmdType(RecoverLease);
+      omClientResponse = new OMRecoverLeaseResponse(
+          createErrorOMResponse(omResponse, ex), getBucketLayout());
+    } finally {
+      if (omClientResponse != null) {
+        omClientResponse.setFlushFuture(
+            ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                transactionLogIndex));
+      }
+      if (acquiredLock) {
+        omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+            bucketName);
+      }
+    }
+
+    // Audit Log outside the lock
+    auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+        OMAction.RECOVER_LEASE, auditMap, exception,
+        getOmRequest().getUserInfo()));
+
+    return omClientResponse;
+  }
+
+  private String doWork(OzoneManager ozoneManager,
+      RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+      throws IOException {
+
+    final long volumeId = omMetadataManager.getVolumeId(volumeName);
+    final long bucketId = omMetadataManager.getBucketId(
+        volumeName, bucketName);
+    Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+    long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+        pathComponents, keyName, omMetadataManager,
+        "Cannot recover file : " + keyName
+            + " as parent directory doesn't exist");
+    String fileName = OzoneFSUtils.getFileName(keyName);
+    String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+        parentID, fileName);
+
+    OmKeyInfo keyInfo = getKey(dbFileKey);
+    if (keyInfo == null) {
+      throw new OMException("Key:" + keyName + " not found", KEY_NOT_FOUND);
+    }
+    final String clientId = keyInfo.getMetadata().get(
+        OzoneConsts.HSYNC_CLIENT_ID);
+    if (clientId == null) {
+      LOG.warn("Key:" + keyName + " is closed");
+    }
+    String openKeyEntryName = getOpenFileEntryName(volumeId, bucketId,
+        parentID, fileName);
+    if (openKeyEntryName != null) {
+      checkFileState(keyInfo, openKeyEntryName);
+      commitKey(dbFileKey, keyInfo, ozoneManager, transactionLogIndex);
+      removeOpenKey(openKeyEntryName, transactionLogIndex);
+    }
+
+    return openKeyEntryName;
+  }
+
+  private OmKeyInfo getKey(String dbOzoneKey) throws IOException {
+    return omMetadataManager.getKeyTable(getBucketLayout()).get(dbOzoneKey);
+  }
+
+  private String getOpenFileEntryName(long volumeId, long bucketId,
+      long parentObjectId, String fileName) throws IOException {
+    String openFileEntryName = null;
+    String dbOpenKeyPrefix =
+        omMetadataManager.getOpenFileNamePrefix(volumeId, bucketId,
+            parentObjectId, fileName);
+    try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
+        iter = omMetadataManager.getOpenKeyTable(getBucketLayout())
+        .iterator(dbOpenKeyPrefix)) {
+
+      while (iter.hasNext()) {
+        if (openFileEntryName != null) {
+          throw new IOException("Found more than two keys in the" +
+            " open key table for file" + fileName);
+        }
+        openFileEntryName = iter.next().getKey();
+      }
+    }
+    /*if (openFileEntryName == null) {
+      throw new IOException("Unable to find the corresponding key in " +
+        "the open key table for file " + fileName);
+    }*/
+    return openFileEntryName;
+  }
+
+  private void checkFileState(OmKeyInfo keyInfo, String openKeyEntryName)
+      throws IOException {
+
+    // if file is closed, do nothing and return right away.
+    if (openKeyEntryName.isEmpty()) {
+      return; // ("File is closed");
+    }
+    // if file is open, no sync, fail right away.
+    if (keyInfo == null) {
+      throw new IOException("file is open but not yet sync'ed");

Review Comment:
   Use `OMException`.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,281 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private OMMetadataManager omMetadataManager;
+
+  public OMRecoverLeaseRequest(OMRequest omRequest) {
+    super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    Preconditions.checkNotNull(recoverLeaseRequest);
+    volumeName = recoverLeaseRequest.getVolumeName();
+    bucketName = recoverLeaseRequest.getBucketName();
+    keyName = recoverLeaseRequest.getKeyName();
+  }
+
+  @Override
+  public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    String keyPath = recoverLeaseRequest.getKeyName();
+    String normalizedKeyPath =
+        validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+    return getOmRequest().toBuilder()
+        .setRecoverLeaseRequest(
+            recoverLeaseRequest.toBuilder()
+                .setKeyName(normalizedKeyPath))
+        .build();
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+      long transactionLogIndex,
+      OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+    Preconditions.checkNotNull(recoverLeaseRequest);
+
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.VOLUME, volumeName);
+    auditMap.put(OzoneConsts.BUCKET, bucketName);
+    auditMap.put(OzoneConsts.KEY, keyName);
+
+    OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+        getOmRequest());
+
+    omMetadataManager = ozoneManager.getMetadataManager();
+    OMClientResponse omClientResponse = null;
+    IOException exception = null;
+    // increment metric
+    OMMetrics omMetrics = ozoneManager.getMetrics();
+
+    boolean acquiredLock = false;
+    try {
+      // check ACL
+      checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+          IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+      // acquire lock
+      acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+          volumeName, bucketName);
+
+      validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+      String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+          transactionLogIndex);
+
+      // Prepare response
+      boolean responseCode = true;
+      omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+              .setResponse(responseCode).build())
+          .setCmdType(RecoverLease);
+      omClientResponse =
+          new OMRecoverLeaseResponse(omResponse.build(), getBucketLayout(),
+              openKeyEntryName);
+      omMetrics.incNumRecoverLease();
+      LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+          bucketName, keyName);
+    } catch (IOException ex) {
+      LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+          volumeName, bucketName, keyName, ex);
+      exception = ex;
+      omMetrics.incNumRecoverLeaseFails();
+      omResponse.setCmdType(RecoverLease);
+      omClientResponse = new OMRecoverLeaseResponse(
+          createErrorOMResponse(omResponse, ex), getBucketLayout());
+    } finally {
+      if (omClientResponse != null) {
+        omClientResponse.setFlushFuture(
+            ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                transactionLogIndex));
+      }
+      if (acquiredLock) {
+        omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+            bucketName);
+      }
+    }
+
+    // Audit Log outside the lock
+    auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+        OMAction.RECOVER_LEASE, auditMap, exception,
+        getOmRequest().getUserInfo()));
+
+    return omClientResponse;
+  }
+
+  private String doWork(OzoneManager ozoneManager,
+      RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+      throws IOException {
+
+    final long volumeId = omMetadataManager.getVolumeId(volumeName);
+    final long bucketId = omMetadataManager.getBucketId(
+        volumeName, bucketName);
+    Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+    long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+        pathComponents, keyName, omMetadataManager,
+        "Cannot recover file : " + keyName
+            + " as parent directory doesn't exist");
+    String fileName = OzoneFSUtils.getFileName(keyName);
+    String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+        parentID, fileName);
+
+    OmKeyInfo keyInfo = getKey(dbFileKey);
+    if (keyInfo == null) {
+      throw new OMException("Key:" + keyName + " not found", KEY_NOT_FOUND);
+    }
+    final String clientId = keyInfo.getMetadata().get(

Review Comment:
   Sorry, we should use `remove(..)` instead of `get(..)`.  Then, `clientId` in the metadata will be removed when will update the cache and KeyTable later.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/file/OMRecoverLeaseResponse.java:
##########
@@ -0,0 +1,54 @@
+package org.apache.hadoop.ozone.om.response.file;
+
+import org.apache.hadoop.hdds.utils.db.BatchOperation;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.response.CleanupTableInfo;
+import org.apache.hadoop.ozone.om.response.key.OmKeyResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+
+import javax.annotation.Nonnull;
+import java.io.IOException;
+
+import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.FILE_TABLE;
+import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.OPEN_FILE_TABLE;
+
+/**
+ * Performs tasks for RecoverLease request responses.
+ */
+@CleanupTableInfo(cleanupTables = {FILE_TABLE, OPEN_FILE_TABLE})
+public class OMRecoverLeaseResponse extends OmKeyResponse {
+
+  private String openKeyName;
+  public OMRecoverLeaseResponse(@Nonnull OMResponse omResponse,
+      BucketLayout bucketLayout, String openKeyName) {
+    super(omResponse, bucketLayout);
+    this.openKeyName = openKeyName;
+  }
+
+  /**
+   * For when the request is not successful.
+   * For a successful request, the other constructor should be used.
+   */
+  public OMRecoverLeaseResponse(@Nonnull OMResponse omResponse,
+      @Nonnull BucketLayout bucketLayout) {
+    super(omResponse, bucketLayout);
+    checkStatusNotOK();
+  }
+
+  @Override
+  protected void addToDBBatch(OMMetadataManager omMetadataManager,
+      BatchOperation batchOperation) throws IOException {
+    // Delete from OpenKey table
+    if (openKeyName != null) {

Review Comment:
   We also needs to update the KeyTable to update the `OmKeyInfo` for removing the clientId from metadata.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/file/OMRecoverLeaseResponse.java:
##########
@@ -0,0 +1,54 @@
+package org.apache.hadoop.ozone.om.response.file;
+
+import org.apache.hadoop.hdds.utils.db.BatchOperation;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.response.CleanupTableInfo;
+import org.apache.hadoop.ozone.om.response.key.OmKeyResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+
+import javax.annotation.Nonnull;
+import java.io.IOException;
+
+import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.FILE_TABLE;
+import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.OPEN_FILE_TABLE;
+
+/**
+ * Performs tasks for RecoverLease request responses.
+ */
+@CleanupTableInfo(cleanupTables = {FILE_TABLE, OPEN_FILE_TABLE})
+public class OMRecoverLeaseResponse extends OmKeyResponse {
+
+  private String openKeyName;
+  public OMRecoverLeaseResponse(@Nonnull OMResponse omResponse,
+      BucketLayout bucketLayout, String openKeyName) {

Review Comment:
   We should include the updated `OmKeyInfo` (for removing the clientId from the metadata).



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,281 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private OMMetadataManager omMetadataManager;
+
+  public OMRecoverLeaseRequest(OMRequest omRequest) {
+    super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    Preconditions.checkNotNull(recoverLeaseRequest);
+    volumeName = recoverLeaseRequest.getVolumeName();
+    bucketName = recoverLeaseRequest.getBucketName();
+    keyName = recoverLeaseRequest.getKeyName();
+  }
+
+  @Override
+  public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    String keyPath = recoverLeaseRequest.getKeyName();
+    String normalizedKeyPath =
+        validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+    return getOmRequest().toBuilder()
+        .setRecoverLeaseRequest(
+            recoverLeaseRequest.toBuilder()
+                .setKeyName(normalizedKeyPath))
+        .build();
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+      long transactionLogIndex,
+      OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+    Preconditions.checkNotNull(recoverLeaseRequest);
+
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.VOLUME, volumeName);
+    auditMap.put(OzoneConsts.BUCKET, bucketName);
+    auditMap.put(OzoneConsts.KEY, keyName);
+
+    OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+        getOmRequest());
+
+    omMetadataManager = ozoneManager.getMetadataManager();
+    OMClientResponse omClientResponse = null;
+    IOException exception = null;
+    // increment metric
+    OMMetrics omMetrics = ozoneManager.getMetrics();
+
+    boolean acquiredLock = false;
+    try {
+      // check ACL
+      checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+          IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+      // acquire lock
+      acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+          volumeName, bucketName);
+
+      validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+      String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+          transactionLogIndex);
+
+      // Prepare response
+      boolean responseCode = true;
+      omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+              .setResponse(responseCode).build())
+          .setCmdType(RecoverLease);
+      omClientResponse =
+          new OMRecoverLeaseResponse(omResponse.build(), getBucketLayout(),
+              openKeyEntryName);
+      omMetrics.incNumRecoverLease();
+      LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+          bucketName, keyName);
+    } catch (IOException ex) {
+      LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+          volumeName, bucketName, keyName, ex);
+      exception = ex;
+      omMetrics.incNumRecoverLeaseFails();
+      omResponse.setCmdType(RecoverLease);
+      omClientResponse = new OMRecoverLeaseResponse(
+          createErrorOMResponse(omResponse, ex), getBucketLayout());
+    } finally {
+      if (omClientResponse != null) {
+        omClientResponse.setFlushFuture(
+            ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                transactionLogIndex));
+      }
+      if (acquiredLock) {
+        omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+            bucketName);
+      }
+    }
+
+    // Audit Log outside the lock
+    auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+        OMAction.RECOVER_LEASE, auditMap, exception,
+        getOmRequest().getUserInfo()));
+
+    return omClientResponse;
+  }
+
+  private String doWork(OzoneManager ozoneManager,
+      RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+      throws IOException {
+
+    final long volumeId = omMetadataManager.getVolumeId(volumeName);
+    final long bucketId = omMetadataManager.getBucketId(
+        volumeName, bucketName);
+    Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+    long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+        pathComponents, keyName, omMetadataManager,
+        "Cannot recover file : " + keyName
+            + " as parent directory doesn't exist");
+    String fileName = OzoneFSUtils.getFileName(keyName);
+    String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+        parentID, fileName);
+
+    OmKeyInfo keyInfo = getKey(dbFileKey);
+    if (keyInfo == null) {
+      throw new OMException("Key:" + keyName + " not found", KEY_NOT_FOUND);
+    }
+    final String clientId = keyInfo.getMetadata().get(
+        OzoneConsts.HSYNC_CLIENT_ID);
+    if (clientId == null) {
+      LOG.warn("Key:" + keyName + " is closed");
+    }
+    String openKeyEntryName = getOpenFileEntryName(volumeId, bucketId,
+        parentID, fileName);
+    if (openKeyEntryName != null) {
+      checkFileState(keyInfo, openKeyEntryName);
+      commitKey(dbFileKey, keyInfo, ozoneManager, transactionLogIndex);
+      removeOpenKey(openKeyEntryName, transactionLogIndex);
+    }
+
+    return openKeyEntryName;
+  }
+
+  private OmKeyInfo getKey(String dbOzoneKey) throws IOException {
+    return omMetadataManager.getKeyTable(getBucketLayout()).get(dbOzoneKey);
+  }
+
+  private String getOpenFileEntryName(long volumeId, long bucketId,
+      long parentObjectId, String fileName) throws IOException {
+    String openFileEntryName = null;
+    String dbOpenKeyPrefix =
+        omMetadataManager.getOpenFileNamePrefix(volumeId, bucketId,
+            parentObjectId, fileName);
+    try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
+        iter = omMetadataManager.getOpenKeyTable(getBucketLayout())
+        .iterator(dbOpenKeyPrefix)) {
+
+      while (iter.hasNext()) {
+        if (openFileEntryName != null) {
+          throw new IOException("Found more than two keys in the" +
+            " open key table for file" + fileName);
+        }
+        openFileEntryName = iter.next().getKey();
+      }
+    }
+    /*if (openFileEntryName == null) {
+      throw new IOException("Unable to find the corresponding key in " +
+        "the open key table for file " + fileName);
+    }*/
+    return openFileEntryName;
+  }
+
+  private void checkFileState(OmKeyInfo keyInfo, String openKeyEntryName)
+      throws IOException {
+
+    // if file is closed, do nothing and return right away.
+    if (openKeyEntryName.isEmpty()) {
+      return; // ("File is closed");
+    }
+    // if file is open, no sync, fail right away.
+    if (keyInfo == null) {
+      throw new IOException("file is open but not yet sync'ed");
+    }
+  }
+
+  private void commitKey(String dbOzoneKey, OmKeyInfo omKeyInfo,
+      OzoneManager ozoneManager, long transactionLogIndex) throws IOException {
+    omKeyInfo.setModificationTime(Time.now());
+    omKeyInfo.setUpdateID(transactionLogIndex, ozoneManager.isRatisEnabled());
+
+    omMetadataManager.getKeyTable(getBucketLayout()).addCacheEntry(
+      new CacheKey<>(dbOzoneKey),
+      new CacheValue<>(Optional.of(omKeyInfo), transactionLogIndex));
+  }
+
+  private void removeOpenKey(String openKeyName, long transactionLogIndex)
+      throws IOException {

Review Comment:
   Remove `throws IOException`.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,281 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private OMMetadataManager omMetadataManager;
+
+  public OMRecoverLeaseRequest(OMRequest omRequest) {
+    super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    Preconditions.checkNotNull(recoverLeaseRequest);
+    volumeName = recoverLeaseRequest.getVolumeName();
+    bucketName = recoverLeaseRequest.getBucketName();
+    keyName = recoverLeaseRequest.getKeyName();
+  }
+
+  @Override
+  public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    String keyPath = recoverLeaseRequest.getKeyName();
+    String normalizedKeyPath =
+        validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+    return getOmRequest().toBuilder()
+        .setRecoverLeaseRequest(
+            recoverLeaseRequest.toBuilder()
+                .setKeyName(normalizedKeyPath))
+        .build();
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+      long transactionLogIndex,
+      OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+    Preconditions.checkNotNull(recoverLeaseRequest);
+
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.VOLUME, volumeName);
+    auditMap.put(OzoneConsts.BUCKET, bucketName);
+    auditMap.put(OzoneConsts.KEY, keyName);
+
+    OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+        getOmRequest());
+
+    omMetadataManager = ozoneManager.getMetadataManager();
+    OMClientResponse omClientResponse = null;
+    IOException exception = null;
+    // increment metric
+    OMMetrics omMetrics = ozoneManager.getMetrics();
+
+    boolean acquiredLock = false;
+    try {
+      // check ACL
+      checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+          IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+      // acquire lock
+      acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+          volumeName, bucketName);
+
+      validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+      String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+          transactionLogIndex);
+
+      // Prepare response
+      boolean responseCode = true;
+      omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+              .setResponse(responseCode).build())
+          .setCmdType(RecoverLease);
+      omClientResponse =
+          new OMRecoverLeaseResponse(omResponse.build(), getBucketLayout(),
+              openKeyEntryName);
+      omMetrics.incNumRecoverLease();
+      LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+          bucketName, keyName);
+    } catch (IOException ex) {
+      LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+          volumeName, bucketName, keyName, ex);
+      exception = ex;
+      omMetrics.incNumRecoverLeaseFails();
+      omResponse.setCmdType(RecoverLease);
+      omClientResponse = new OMRecoverLeaseResponse(
+          createErrorOMResponse(omResponse, ex), getBucketLayout());
+    } finally {
+      if (omClientResponse != null) {
+        omClientResponse.setFlushFuture(
+            ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                transactionLogIndex));
+      }
+      if (acquiredLock) {
+        omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+            bucketName);
+      }
+    }
+
+    // Audit Log outside the lock
+    auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+        OMAction.RECOVER_LEASE, auditMap, exception,
+        getOmRequest().getUserInfo()));
+
+    return omClientResponse;
+  }
+
+  private String doWork(OzoneManager ozoneManager,
+      RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+      throws IOException {
+
+    final long volumeId = omMetadataManager.getVolumeId(volumeName);
+    final long bucketId = omMetadataManager.getBucketId(
+        volumeName, bucketName);
+    Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+    long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+        pathComponents, keyName, omMetadataManager,
+        "Cannot recover file : " + keyName
+            + " as parent directory doesn't exist");
+    String fileName = OzoneFSUtils.getFileName(keyName);
+    String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+        parentID, fileName);
+
+    OmKeyInfo keyInfo = getKey(dbFileKey);
+    if (keyInfo == null) {
+      throw new OMException("Key:" + keyName + " not found", KEY_NOT_FOUND);
+    }
+    final String clientId = keyInfo.getMetadata().get(
+        OzoneConsts.HSYNC_CLIENT_ID);
+    if (clientId == null) {
+      LOG.warn("Key:" + keyName + " is closed");
+    }
+    String openKeyEntryName = getOpenFileEntryName(volumeId, bucketId,
+        parentID, fileName);
+    if (openKeyEntryName != null) {
+      checkFileState(keyInfo, openKeyEntryName);
+      commitKey(dbFileKey, keyInfo, ozoneManager, transactionLogIndex);
+      removeOpenKey(openKeyEntryName, transactionLogIndex);
+    }
+
+    return openKeyEntryName;
+  }
+
+  private OmKeyInfo getKey(String dbOzoneKey) throws IOException {
+    return omMetadataManager.getKeyTable(getBucketLayout()).get(dbOzoneKey);
+  }
+
+  private String getOpenFileEntryName(long volumeId, long bucketId,
+      long parentObjectId, String fileName) throws IOException {
+    String openFileEntryName = null;
+    String dbOpenKeyPrefix =
+        omMetadataManager.getOpenFileNamePrefix(volumeId, bucketId,
+            parentObjectId, fileName);
+    try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
+        iter = omMetadataManager.getOpenKeyTable(getBucketLayout())
+        .iterator(dbOpenKeyPrefix)) {
+
+      while (iter.hasNext()) {
+        if (openFileEntryName != null) {
+          throw new IOException("Found more than two keys in the" +
+            " open key table for file" + fileName);
+        }
+        openFileEntryName = iter.next().getKey();
+      }
+    }
+    /*if (openFileEntryName == null) {
+      throw new IOException("Unable to find the corresponding key in " +
+        "the open key table for file " + fileName);
+    }*/
+    return openFileEntryName;
+  }
+
+  private void checkFileState(OmKeyInfo keyInfo, String openKeyEntryName)
+      throws IOException {
+
+    // if file is closed, do nothing and return right away.
+    if (openKeyEntryName.isEmpty()) {
+      return; // ("File is closed");
+    }
+    // if file is open, no sync, fail right away.
+    if (keyInfo == null) {
+      throw new IOException("file is open but not yet sync'ed");
+    }
+  }
+
+  private void commitKey(String dbOzoneKey, OmKeyInfo omKeyInfo,
+      OzoneManager ozoneManager, long transactionLogIndex) throws IOException {
+    omKeyInfo.setModificationTime(Time.now());
+    omKeyInfo.setUpdateID(transactionLogIndex, ozoneManager.isRatisEnabled());
+
+    omMetadataManager.getKeyTable(getBucketLayout()).addCacheEntry(
+      new CacheKey<>(dbOzoneKey),
+      new CacheValue<>(Optional.of(omKeyInfo), transactionLogIndex));
+  }
+
+  private void removeOpenKey(String openKeyName, long transactionLogIndex)
+      throws IOException {
+    omMetadataManager.getOpenKeyTable(getBucketLayout()).addCacheEntry(
+      new CacheKey<>(openKeyName),
+      new CacheValue<>(Optional.absent(), transactionLogIndex));

Review Comment:
   Use `CacheValue.get(transactionLogIndex)`.



-- 
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] jojochuang commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1157570661


##########
hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java:
##########
@@ -505,6 +505,19 @@ String getOpenFileName(long volumeId, long bucketId,
    */
   String getRenameKey(String volume, String bucket, long objectID);
 
+  /**
+   * Returns DB key name of an open file in OM metadata store. Should be
+   * #open# prefix followed by actual leaf node name.
+   *
+   * @param volumeId       - ID of the volume
+   * @param bucketId       - ID of the bucket
+   * @param parentObjectId - parent object Id
+   * @param fileName       - file name
+   * @return DB directory key as String.
+   */
+  String getOpenFileNamePrefix(long volumeId, long bucketId,

Review Comment:
   Actually this is a miss on my part. This is no longer needed. Will remove it in the next iteration.



-- 
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] sumitagrawl commented on pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on PR #4234:
URL: https://github.com/apache/ozone/pull/4234#issuecomment-1422095875

   @jojochuang Also I am not able to identify use case of this API? as option from CLI is not provided.


-- 
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] jojochuang commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1119353505


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/utils/OzoneManagerRatisUtils.java:
##########
@@ -216,7 +218,17 @@ public static OMClientRequest createClientRequest(OMRequest omRequest,
             omRequest.getDeleteOpenKeysRequest().getBucketLayout());
       }
       return new OMOpenKeysDeleteRequest(omRequest, bktLayout);
-
+    case RecoverLease: {
+      volumeName = omRequest.getRecoverLeaseRequest().getVolumeName();
+      bucketName = omRequest.getRecoverLeaseRequest().getBucketName();
+      bucketLayout =
+              getBucketLayout(ozoneManager.getMetadataManager(), volumeName, bucketName);
+      if (bucketLayout != BucketLayout.FILE_SYSTEM_OPTIMIZED) {

Review Comment:
   No... i don't think it's a good idea to break the existing object store semantics. No object store support a client lease recovery semantics. This is purely meant for HDFS-like semantics for existing HDFS applications to migrate over to Ozone, so I believe it is exclusively FSO.



-- 
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] szetszwo commented on pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #4234:
URL: https://github.com/apache/ozone/pull/4234#issuecomment-1479235219

   @jojochuang , there are some conflicts.  Could you sync this with the master branch?


-- 
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] jojochuang commented on pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #4234:
URL: https://github.com/apache/ozone/pull/4234#issuecomment-1479798909

   Updated code to resolve conflict, and to fix compilation error introduce by HDDS-7958.


-- 
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] sumitagrawl commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1151763526


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,281 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private OMMetadataManager omMetadataManager;
+
+  public OMRecoverLeaseRequest(OMRequest omRequest) {
+    super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    Preconditions.checkNotNull(recoverLeaseRequest);
+    volumeName = recoverLeaseRequest.getVolumeName();
+    bucketName = recoverLeaseRequest.getBucketName();
+    keyName = recoverLeaseRequest.getKeyName();
+  }
+
+  @Override
+  public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    String keyPath = recoverLeaseRequest.getKeyName();
+    String normalizedKeyPath =
+        validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+    return getOmRequest().toBuilder()
+        .setRecoverLeaseRequest(
+            recoverLeaseRequest.toBuilder()
+                .setKeyName(normalizedKeyPath))
+        .build();
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+      long transactionLogIndex,
+      OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+    Preconditions.checkNotNull(recoverLeaseRequest);
+
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.VOLUME, volumeName);
+    auditMap.put(OzoneConsts.BUCKET, bucketName);
+    auditMap.put(OzoneConsts.KEY, keyName);
+
+    OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+        getOmRequest());
+
+    omMetadataManager = ozoneManager.getMetadataManager();
+    OMClientResponse omClientResponse = null;
+    IOException exception = null;
+    // increment metric
+    OMMetrics omMetrics = ozoneManager.getMetrics();
+
+    boolean acquiredLock = false;
+    try {
+      // check ACL
+      checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+          IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+      // acquire lock
+      acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+          volumeName, bucketName);
+
+      validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+      String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+          transactionLogIndex);
+
+      // Prepare response
+      boolean responseCode = true;
+      omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+              .setResponse(responseCode).build())
+          .setCmdType(RecoverLease);
+      omClientResponse =
+          new OMRecoverLeaseResponse(omResponse.build(), getBucketLayout(),
+              openKeyEntryName);
+      omMetrics.incNumRecoverLease();
+      LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+          bucketName, keyName);
+    } catch (IOException ex) {
+      LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+          volumeName, bucketName, keyName, ex);
+      exception = ex;
+      omMetrics.incNumRecoverLeaseFails();
+      omResponse.setCmdType(RecoverLease);
+      omClientResponse = new OMRecoverLeaseResponse(
+          createErrorOMResponse(omResponse, ex), getBucketLayout());
+    } finally {
+      if (omClientResponse != null) {
+        omClientResponse.setFlushFuture(
+            ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                transactionLogIndex));
+      }
+      if (acquiredLock) {
+        omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+            bucketName);
+      }
+    }
+
+    // Audit Log outside the lock
+    auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+        OMAction.RECOVER_LEASE, auditMap, exception,
+        getOmRequest().getUserInfo()));
+
+    return omClientResponse;
+  }
+
+  private String doWork(OzoneManager ozoneManager,
+      RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+      throws IOException {
+
+    final long volumeId = omMetadataManager.getVolumeId(volumeName);
+    final long bucketId = omMetadataManager.getBucketId(
+        volumeName, bucketName);
+    Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+    long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+        pathComponents, keyName, omMetadataManager,
+        "Cannot recover file : " + keyName
+            + " as parent directory doesn't exist");
+    String fileName = OzoneFSUtils.getFileName(keyName);
+    String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+        parentID, fileName);
+
+    OmKeyInfo keyInfo = getKey(dbFileKey);

Review Comment:
   @jojochuang I think key will be present only in openKeyTable (except case of Hsync), so mostly this will be null. This may fail for normal case.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -0,0 +1,281 @@
+/**
+ * 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.ozone.om.request.file;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.TableIterator;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMetrics;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.key.OMKeyRequest;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
+import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.file.OMRecoverLeaseResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMResponse;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .RecoverLeaseResponse;
+
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.util.Time;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
+import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
+import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
+        .Type.RecoverLease;
+
+/**
+ * Perform actions for RecoverLease requests.
+ */
+public class OMRecoverLeaseRequest extends OMKeyRequest {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(OMRecoverLeaseRequest.class);
+
+  private String volumeName;
+  private String bucketName;
+  private String keyName;
+
+  private OMMetadataManager omMetadataManager;
+
+  public OMRecoverLeaseRequest(OMRequest omRequest) {
+    super(omRequest, BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    Preconditions.checkNotNull(recoverLeaseRequest);
+    volumeName = recoverLeaseRequest.getVolumeName();
+    bucketName = recoverLeaseRequest.getBucketName();
+    keyName = recoverLeaseRequest.getKeyName();
+  }
+
+  @Override
+  public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+
+    String keyPath = recoverLeaseRequest.getKeyName();
+    String normalizedKeyPath =
+        validateAndNormalizeKey(ozoneManager.getEnableFileSystemPaths(),
+            keyPath, getBucketLayout());
+
+    return getOmRequest().toBuilder()
+        .setRecoverLeaseRequest(
+            recoverLeaseRequest.toBuilder()
+                .setKeyName(normalizedKeyPath))
+        .build();
+  }
+
+  @Override
+  public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
+      long transactionLogIndex,
+      OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
+    RecoverLeaseRequest recoverLeaseRequest = getOmRequest()
+        .getRecoverLeaseRequest();
+    Preconditions.checkNotNull(recoverLeaseRequest);
+
+    Map<String, String> auditMap = new LinkedHashMap<>();
+    auditMap.put(OzoneConsts.VOLUME, volumeName);
+    auditMap.put(OzoneConsts.BUCKET, bucketName);
+    auditMap.put(OzoneConsts.KEY, keyName);
+
+    OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
+        getOmRequest());
+
+    omMetadataManager = ozoneManager.getMetadataManager();
+    OMClientResponse omClientResponse = null;
+    IOException exception = null;
+    // increment metric
+    OMMetrics omMetrics = ozoneManager.getMetrics();
+
+    boolean acquiredLock = false;
+    try {
+      // check ACL
+      checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
+          IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
+
+      // acquire lock
+      acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
+          volumeName, bucketName);
+
+      validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
+
+      String openKeyEntryName = doWork(ozoneManager, recoverLeaseRequest,
+          transactionLogIndex);
+
+      // Prepare response
+      boolean responseCode = true;
+      omResponse.setRecoverLeaseResponse(RecoverLeaseResponse.newBuilder()
+              .setResponse(responseCode).build())
+          .setCmdType(RecoverLease);
+      omClientResponse =
+          new OMRecoverLeaseResponse(omResponse.build(), getBucketLayout(),
+              openKeyEntryName);
+      omMetrics.incNumRecoverLease();
+      LOG.debug("Key recovered. Volume:{}, Bucket:{}, Key:{}", volumeName,
+          bucketName, keyName);
+    } catch (IOException ex) {
+      LOG.error("Fail for recovering lease. Volume:{}, Bucket:{}, Key:{}",
+          volumeName, bucketName, keyName, ex);
+      exception = ex;
+      omMetrics.incNumRecoverLeaseFails();
+      omResponse.setCmdType(RecoverLease);
+      omClientResponse = new OMRecoverLeaseResponse(
+          createErrorOMResponse(omResponse, ex), getBucketLayout());
+    } finally {
+      if (omClientResponse != null) {
+        omClientResponse.setFlushFuture(
+            ozoneManagerDoubleBufferHelper.add(omClientResponse,
+                transactionLogIndex));
+      }
+      if (acquiredLock) {
+        omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+            bucketName);
+      }
+    }
+
+    // Audit Log outside the lock
+    auditLog(ozoneManager.getAuditLogger(), buildAuditMessage(
+        OMAction.RECOVER_LEASE, auditMap, exception,
+        getOmRequest().getUserInfo()));
+
+    return omClientResponse;
+  }
+
+  private String doWork(OzoneManager ozoneManager,
+      RecoverLeaseRequest recoverLeaseRequest, long transactionLogIndex)
+      throws IOException {
+
+    final long volumeId = omMetadataManager.getVolumeId(volumeName);
+    final long bucketId = omMetadataManager.getBucketId(
+        volumeName, bucketName);
+    Iterator<Path> pathComponents = Paths.get(keyName).iterator();
+    long parentID = OMFileRequest.getParentID(volumeId, bucketId,
+        pathComponents, keyName, omMetadataManager,
+        "Cannot recover file : " + keyName
+            + " as parent directory doesn't exist");
+    String fileName = OzoneFSUtils.getFileName(keyName);
+    String dbFileKey = omMetadataManager.getOzonePathKey(volumeId, bucketId,
+        parentID, fileName);
+
+    OmKeyInfo keyInfo = getKey(dbFileKey);
+    if (keyInfo == null) {
+      throw new OMException("Key:" + keyName + " not found", KEY_NOT_FOUND);
+    }
+    final String clientId = keyInfo.getMetadata().get(
+        OzoneConsts.HSYNC_CLIENT_ID);
+    if (clientId == null) {
+      LOG.warn("Key:" + keyName + " is closed");
+    }
+    String openKeyEntryName = getOpenFileEntryName(volumeId, bucketId,
+        parentID, fileName);
+    if (openKeyEntryName != null) {
+      checkFileState(keyInfo, openKeyEntryName);
+      commitKey(dbFileKey, keyInfo, ozoneManager, transactionLogIndex);
+      removeOpenKey(openKeyEntryName, transactionLogIndex);
+    }
+
+    return openKeyEntryName;
+  }
+
+  private OmKeyInfo getKey(String dbOzoneKey) throws IOException {
+    return omMetadataManager.getKeyTable(getBucketLayout()).get(dbOzoneKey);
+  }
+
+  private String getOpenFileEntryName(long volumeId, long bucketId,
+      long parentObjectId, String fileName) throws IOException {
+    String openFileEntryName = null;
+    String dbOpenKeyPrefix =
+        omMetadataManager.getOpenFileNamePrefix(volumeId, bucketId,
+            parentObjectId, fileName);
+    try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
+        iter = omMetadataManager.getOpenKeyTable(getBucketLayout())
+        .iterator(dbOpenKeyPrefix)) {
+
+      while (iter.hasNext()) {
+        if (openFileEntryName != null) {
+          throw new IOException("Found more than two keys in the" +
+            " open key table for file" + fileName);
+        }
+        openFileEntryName = iter.next().getKey();
+      }
+    }
+    /*if (openFileEntryName == null) {
+      throw new IOException("Unable to find the corresponding key in " +
+        "the open key table for file " + fileName);
+    }*/
+    return openFileEntryName;
+  }
+
+  private void checkFileState(OmKeyInfo keyInfo, String openKeyEntryName)
+      throws IOException {
+
+    // if file is closed, do nothing and return right away.
+    if (openKeyEntryName.isEmpty()) {
+      return; // ("File is closed");
+    }
+    // if file is open, no sync, fail right away.
+    if (keyInfo == null) {
+      throw new IOException("file is open but not yet sync'ed");
+    }
+  }
+
+  private void commitKey(String dbOzoneKey, OmKeyInfo omKeyInfo,
+      OzoneManager ozoneManager, long transactionLogIndex) throws IOException {
+    omKeyInfo.setModificationTime(Time.now());
+    omKeyInfo.setUpdateID(transactionLogIndex, ozoneManager.isRatisEnabled());

Review Comment:
   Please check impact of multi-part key present in openKey table for FSO bucket type, its commit have different behavior and can not commit in that case.



-- 
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] jojochuang commented on a diff in pull request #4234: HDDS-7769. Implement client initiated lease recovery

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #4234:
URL: https://github.com/apache/ozone/pull/4234#discussion_r1157978546


##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java:
##########
@@ -983,4 +983,16 @@ EchoRPCResponse echoRPCReq(byte[] payloadReq,
                              int payloadSizeResp)
           throws IOException;
 
+
+  /**
+   * Start the lease recovery of a file.
+   *
+   * @param volumeName - The volume name.
+   * @param bucketName - The bucket name.
+   * @param keyName - The key user want to recover.
+   * @return true if the file is already closed
+   * @throws IOException if an error occurs
+   */
+  boolean recoverLease(String volumeName, String bucketName,
+                              String keyName) throws IOException;

Review Comment:
   Thanks for the information. I wasn't aware of it, simply following the past implementations.
   OzoneManagerClientProtocol looks short. Are we supposed to move all client to OM communications over to this interface, and what about the existing APIs?



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