From 2fbf3e0cf71c1841cfc210884ccb7a30f68c538e Mon Sep 17 00:00:00 2001 From: ashishk Date: Thu, 25 Jan 2024 16:21:39 +0530 Subject: [PATCH 1/2] HDDS-10190. Handle lease recovery for file without blocks. --- .../hadoop/fs/ozone/TestLeaseRecovery.java | 21 ++++++++++++ .../request/file/OMRecoverLeaseRequest.java | 32 ++++++++++--------- .../hadoop/fs/ozone/OzoneFileSystem.java | 18 ++++++----- .../fs/ozone/RootedOzoneFileSystem.java | 18 ++++++----- .../hadoop/fs/ozone/OzoneFileSystem.java | 18 ++++++----- .../fs/ozone/RootedOzoneFileSystem.java | 18 ++++++----- 6 files changed, 78 insertions(+), 47 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestLeaseRecovery.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestLeaseRecovery.java index 68c2d43471db..a40c7f52752a 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestLeaseRecovery.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestLeaseRecovery.java @@ -433,6 +433,27 @@ public void testRecoverWrongFile() throws Exception { } } + @Test + public void testRecoveryWithoutBlocks() throws Exception { + RootedOzoneFileSystem fs = (RootedOzoneFileSystem)FileSystem.get(conf); + + final FSDataOutputStream stream = fs.create(file, true); + try { + stream.hsync(); + assertFalse(fs.isFileClosed(file)); + + int count = 0; + while (count++ < 15 && !fs.recoverLease(file)) { + Thread.sleep(1000); + } + // The lease should have been recovered. + assertTrue(fs.isFileClosed(file), "File should be closed"); + + } finally { + closeIgnoringKeyNotFound(stream); + } + } + private void verifyData(byte[] data, int dataSize, Path filePath, RootedOzoneFileSystem fs) throws IOException { try (FSDataInputStream fdis = fs.open(filePath)) { int bufferSize = dataSize > data.length ? dataSize / 2 : dataSize; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java index addcc54977fc..6d997ed272ad 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java @@ -248,29 +248,31 @@ private RecoverLeaseResponse doWork(OzoneManager ozoneManager, List keyLocationInfoList = keyLatestVersionLocations.getLocationList(); OmKeyLocationInfoGroup openKeyLatestVersionLocations = openKeyInfo.getLatestVersionLocations(); List openKeyLocationInfoList = openKeyLatestVersionLocations.getLocationList(); - OmKeyLocationInfo finalBlock; + + OmKeyLocationInfo finalBlock = null; boolean returnKeyInfo = true; if (openKeyLocationInfoList.size() > keyLocationInfoList.size() && - openKeyModificationTime > keyInfo.getModificationTime()) { + openKeyModificationTime > keyInfo.getModificationTime() && + openKeyLocationInfoList.size() > 0) { finalBlock = openKeyLocationInfoList.get(openKeyLocationInfoList.size() - 1); returnKeyInfo = false; - } else { + } else if (keyLocationInfoList.size() > 0) { finalBlock = keyLocationInfoList.get(keyLocationInfoList.size() - 1); } - - // set token to last block if enabled - if (ozoneManager.isGrpcBlockTokenEnabled()) { - String remoteUser = getRemoteUser().getShortUserName(); - OzoneBlockTokenSecretManager secretManager = ozoneManager.getBlockTokenSecretManager(); - finalBlock.setToken(secretManager.generateToken(remoteUser, finalBlock.getBlockID(), - EnumSet.of(READ, WRITE), finalBlock.getLength())); + if (finalBlock != null) { + // set token to last block if enabled + if (ozoneManager.isGrpcBlockTokenEnabled()) { + String remoteUser = getRemoteUser().getShortUserName(); + OzoneBlockTokenSecretManager secretManager = ozoneManager.getBlockTokenSecretManager(); + finalBlock.setToken(secretManager.generateToken(remoteUser, finalBlock.getBlockID(), + EnumSet.of(READ, WRITE), finalBlock.getLength())); + } + // refresh last block pipeline + ContainerWithPipeline containerWithPipeline = + ozoneManager.getScmClient().getContainerClient().getContainerWithPipeline(finalBlock.getContainerID()); + finalBlock.setPipeline(containerWithPipeline.getPipeline()); } - // refresh last block pipeline - ContainerWithPipeline containerWithPipeline = - ozoneManager.getScmClient().getContainerClient().getContainerWithPipeline(finalBlock.getContainerID()); - finalBlock.setPipeline(containerWithPipeline.getPipeline()); - RecoverLeaseResponse.Builder rb = RecoverLeaseResponse.newBuilder(); rb.setKeyInfo(returnKeyInfo ? keyInfo.getNetworkProtobuf(getOmRequest().getVersion(), true) : openKeyInfo.getNetworkProtobuf(getOmRequest().getVersion(), true)); diff --git a/hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java b/hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java index 415801a78970..26687e29f62c 100644 --- a/hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java +++ b/hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java @@ -155,15 +155,17 @@ public boolean recoverLease(Path f) throws IOException { // finalize the final block and get block length List locationInfoList = keyInfo.getLatestVersionLocations().getLocationList(); - OmKeyLocationInfo block = locationInfoList.get(locationInfoList.size() - 1); - try { - block.setLength(getAdapter().finalizeBlock(block)); - } catch (Throwable e) { - if (!forceRecovery) { - throw e; + if (locationInfoList.size() > 0) { + OmKeyLocationInfo block = locationInfoList.get(locationInfoList.size() - 1); + try { + block.setLength(getAdapter().finalizeBlock(block)); + } catch (Throwable e) { + if (!forceRecovery) { + throw e; + } + LOG.warn("Failed to finalize block. Continue to recover the file since {} is enabled.", + FORCE_LEASE_RECOVERY_ENV, e); } - LOG.warn("Failed to finalize block. Continue to recover the file since {} is enabled.", - FORCE_LEASE_RECOVERY_ENV, e); } // recover and commit file diff --git a/hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/RootedOzoneFileSystem.java b/hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/RootedOzoneFileSystem.java index f5216cb4516a..0ba358ce308b 100644 --- a/hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/RootedOzoneFileSystem.java +++ b/hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/RootedOzoneFileSystem.java @@ -159,15 +159,17 @@ public boolean recoverLease(final Path f) throws IOException { // finalize the final block and get block length List locationInfoList = keyInfo.getLatestVersionLocations().getLocationList(); - OmKeyLocationInfo block = locationInfoList.get(locationInfoList.size() - 1); - try { - block.setLength(getAdapter().finalizeBlock(block)); - } catch (Throwable e) { - if (!forceRecovery) { - throw e; + if (locationInfoList.size() > 0) { + OmKeyLocationInfo block = locationInfoList.get(locationInfoList.size() - 1); + try { + block.setLength(getAdapter().finalizeBlock(block)); + } catch (Throwable e) { + if (!forceRecovery) { + throw e; + } + LOG.warn("Failed to finalize block. Continue to recover the file since {} is enabled.", + FORCE_LEASE_RECOVERY_ENV, e); } - LOG.warn("Failed to finalize block. Continue to recover the file since {} is enabled.", - FORCE_LEASE_RECOVERY_ENV, e); } // recover and commit file diff --git a/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java b/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java index 415801a78970..26687e29f62c 100644 --- a/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java +++ b/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java @@ -155,15 +155,17 @@ public boolean recoverLease(Path f) throws IOException { // finalize the final block and get block length List locationInfoList = keyInfo.getLatestVersionLocations().getLocationList(); - OmKeyLocationInfo block = locationInfoList.get(locationInfoList.size() - 1); - try { - block.setLength(getAdapter().finalizeBlock(block)); - } catch (Throwable e) { - if (!forceRecovery) { - throw e; + if (locationInfoList.size() > 0) { + OmKeyLocationInfo block = locationInfoList.get(locationInfoList.size() - 1); + try { + block.setLength(getAdapter().finalizeBlock(block)); + } catch (Throwable e) { + if (!forceRecovery) { + throw e; + } + LOG.warn("Failed to finalize block. Continue to recover the file since {} is enabled.", + FORCE_LEASE_RECOVERY_ENV, e); } - LOG.warn("Failed to finalize block. Continue to recover the file since {} is enabled.", - FORCE_LEASE_RECOVERY_ENV, e); } // recover and commit file diff --git a/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/RootedOzoneFileSystem.java b/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/RootedOzoneFileSystem.java index 278462628709..2a84f09c720e 100644 --- a/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/RootedOzoneFileSystem.java +++ b/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/RootedOzoneFileSystem.java @@ -152,15 +152,17 @@ public boolean recoverLease(final Path f) throws IOException { // finalize the final block and get block length List keyLocationInfoList = keyInfo.getLatestVersionLocations().getLocationList(); - OmKeyLocationInfo block = keyLocationInfoList.get(keyLocationInfoList.size() - 1); - try { - block.setLength(getAdapter().finalizeBlock(block)); - } catch (Throwable e) { - if (!forceRecovery) { - throw e; + if (keyLocationInfoList.size() > 0) { + OmKeyLocationInfo block = keyLocationInfoList.get(keyLocationInfoList.size() - 1); + try { + block.setLength(getAdapter().finalizeBlock(block)); + } catch (Throwable e) { + if (!forceRecovery) { + throw e; + } + LOG.warn("Failed to finalize block. Continue to recover the file since {} is enabled.", + FORCE_LEASE_RECOVERY_ENV, e); } - LOG.warn("Failed to finalize block. Continue to recover the file since {} is enabled.", - FORCE_LEASE_RECOVERY_ENV, e); } // recover and commit file From e576685196ca6b9a89ae11fc0f50373c27d8a51d Mon Sep 17 00:00:00 2001 From: ashishk Date: Fri, 26 Jan 2024 09:02:20 +0530 Subject: [PATCH 2/2] Fix review comment --- .../main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java | 2 +- .../java/org/apache/hadoop/fs/ozone/RootedOzoneFileSystem.java | 2 +- .../main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java | 2 +- .../java/org/apache/hadoop/fs/ozone/RootedOzoneFileSystem.java | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java b/hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java index 26687e29f62c..b6cc22bbad09 100644 --- a/hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java +++ b/hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java @@ -155,7 +155,7 @@ public boolean recoverLease(Path f) throws IOException { // finalize the final block and get block length List locationInfoList = keyInfo.getLatestVersionLocations().getLocationList(); - if (locationInfoList.size() > 0) { + if (!locationInfoList.isEmpty()) { OmKeyLocationInfo block = locationInfoList.get(locationInfoList.size() - 1); try { block.setLength(getAdapter().finalizeBlock(block)); diff --git a/hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/RootedOzoneFileSystem.java b/hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/RootedOzoneFileSystem.java index 0ba358ce308b..e6eba955e4d9 100644 --- a/hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/RootedOzoneFileSystem.java +++ b/hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/RootedOzoneFileSystem.java @@ -159,7 +159,7 @@ public boolean recoverLease(final Path f) throws IOException { // finalize the final block and get block length List locationInfoList = keyInfo.getLatestVersionLocations().getLocationList(); - if (locationInfoList.size() > 0) { + if (!locationInfoList.isEmpty()) { OmKeyLocationInfo block = locationInfoList.get(locationInfoList.size() - 1); try { block.setLength(getAdapter().finalizeBlock(block)); diff --git a/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java b/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java index 26687e29f62c..b6cc22bbad09 100644 --- a/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java +++ b/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java @@ -155,7 +155,7 @@ public boolean recoverLease(Path f) throws IOException { // finalize the final block and get block length List locationInfoList = keyInfo.getLatestVersionLocations().getLocationList(); - if (locationInfoList.size() > 0) { + if (!locationInfoList.isEmpty()) { OmKeyLocationInfo block = locationInfoList.get(locationInfoList.size() - 1); try { block.setLength(getAdapter().finalizeBlock(block)); diff --git a/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/RootedOzoneFileSystem.java b/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/RootedOzoneFileSystem.java index 2a84f09c720e..36aa0e5f27c8 100644 --- a/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/RootedOzoneFileSystem.java +++ b/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/RootedOzoneFileSystem.java @@ -152,7 +152,7 @@ public boolean recoverLease(final Path f) throws IOException { // finalize the final block and get block length List keyLocationInfoList = keyInfo.getLatestVersionLocations().getLocationList(); - if (keyLocationInfoList.size() > 0) { + if (!keyLocationInfoList.isEmpty()) { OmKeyLocationInfo block = keyLocationInfoList.get(keyLocationInfoList.size() - 1); try { block.setLength(getAdapter().finalizeBlock(block));