You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "ashishkumar50 (via GitHub)" <gi...@apache.org> on 2024/02/05 08:55:49 UTC
[PR] HDDS-10242. Handle penultimate block finalization. [ozone]
ashishkumar50 opened a new pull request, #6164:
URL: https://github.com/apache/ozone/pull/6164
## What changes were proposed in this pull request?
During leaseRecovery if final block doesn't exist in datanode(when data still not yet flushed to datanode for the last block) but exist in openFileTable. In this case recovery client should finalize the penultimate block and the lease recovery should succeed with the penultimate block length.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10242
## How was this patch tested?
New integration test.
--
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
Re: [PR] HDDS-10242. [hsync] Handle penultimate block finalization. [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #6164:
URL: https://github.com/apache/ozone/pull/6164#discussion_r1483597471
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/LeaseKeyInfo.java:
##########
@@ -0,0 +1,41 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.helpers;
+
+/**
+ * This class represents LeaseKeyInfo
+ * isKeyInfo represents whether key is from openKeyTable or keyTable
+ * true corresponds to keyInfo is from keyTable.
Review Comment:
Actually these two lines of comment should be placed directly above `isKeyInfo` declaration below.
--
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
Re: [PR] HDDS-10242. [hsync] Handle penultimate block finalization. [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #6164:
URL: https://github.com/apache/ozone/pull/6164#discussion_r1483596454
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/LeaseKeyInfo.java:
##########
@@ -0,0 +1,41 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.helpers;
+
+/**
+ * This class represents LeaseKeyInfo
+ * isKeyInfo represents whether key is from openKeyTable or keyTable
+ * true corresponds to keyInfo is from keyTable.
Review Comment:
nit
```suggestion
* isKeyInfo = true indicates keyInfo is from keyTable
* isKeyInfo = false indicates keyInfo is from 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
Re: [PR] HDDS-10242. [hsync] Handle penultimate block finalization. [ozone]
Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on PR #6164:
URL: https://github.com/apache/ozone/pull/6164#issuecomment-1926497778
@ChenSammi @jojochuang Please help to review, thanks.
--
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
Re: [PR] HDDS-10242. [hsync] Handle penultimate block finalization. [ozone]
Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on code in PR #6164:
URL: https://github.com/apache/ozone/pull/6164#discussion_r1483945594
##########
hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java:
##########
@@ -154,25 +157,41 @@ public boolean recoverLease(Path f) throws IOException {
}
// finalize the final block and get block length
- List<OmKeyLocationInfo> locationInfoList = keyInfo.getLatestVersionLocations().getLocationList();
+ List<OmKeyLocationInfo> locationInfoList = leaseKeyInfo.getKeyInfo().getLatestVersionLocations().getLocationList();
if (!locationInfoList.isEmpty()) {
OmKeyLocationInfo block = locationInfoList.get(locationInfoList.size() - 1);
try {
block.setLength(getAdapter().finalizeBlock(block));
} catch (Throwable e) {
- if (!forceRecovery) {
+ if (e instanceof StorageContainerException && (((StorageContainerException) e).getResult().equals(NO_SUCH_BLOCK)
+ || ((StorageContainerException) e).getResult().equals(CONTAINER_NOT_FOUND))
+ && !leaseKeyInfo.getIsKeyInfo() && locationInfoList.size() > 1) {
+ locationInfoList = leaseKeyInfo.getKeyInfo().getLatestVersionLocations().getLocationList().subList(0,
+ locationInfoList.size() - 1);
+ block = locationInfoList.get(locationInfoList.size() - 1);
Review Comment:
This is same but we also use locationInfoList completely during [commit](https://github.com/apache/ozone/blob/c68afdc49e7c4ae44a64aca0a8c48235447a26ab/hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java#L191) so we want to truncate the last element.
--
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
Re: [PR] HDDS-10242. [hsync] Handle penultimate block finalization. [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #6164:
URL: https://github.com/apache/ozone/pull/6164#issuecomment-1933475805
This PR needs a rebase. Sorry.
--
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
Re: [PR] HDDS-10242. [hsync] Handle penultimate block finalization. [ozone]
Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on code in PR #6164:
URL: https://github.com/apache/ozone/pull/6164#discussion_r1483945594
##########
hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java:
##########
@@ -154,25 +157,41 @@ public boolean recoverLease(Path f) throws IOException {
}
// finalize the final block and get block length
- List<OmKeyLocationInfo> locationInfoList = keyInfo.getLatestVersionLocations().getLocationList();
+ List<OmKeyLocationInfo> locationInfoList = leaseKeyInfo.getKeyInfo().getLatestVersionLocations().getLocationList();
if (!locationInfoList.isEmpty()) {
OmKeyLocationInfo block = locationInfoList.get(locationInfoList.size() - 1);
try {
block.setLength(getAdapter().finalizeBlock(block));
} catch (Throwable e) {
- if (!forceRecovery) {
+ if (e instanceof StorageContainerException && (((StorageContainerException) e).getResult().equals(NO_SUCH_BLOCK)
+ || ((StorageContainerException) e).getResult().equals(CONTAINER_NOT_FOUND))
+ && !leaseKeyInfo.getIsKeyInfo() && locationInfoList.size() > 1) {
+ locationInfoList = leaseKeyInfo.getKeyInfo().getLatestVersionLocations().getLocationList().subList(0,
+ locationInfoList.size() - 1);
+ block = locationInfoList.get(locationInfoList.size() - 1);
Review Comment:
This is same but we also use `locationInfoList` completely during [commit](https://github.com/apache/ozone/blob/c68afdc49e7c4ae44a64aca0a8c48235447a26ab/hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java#L191) so we want to truncate the last element.
--
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
Re: [PR] HDDS-10242. [hsync] Handle penultimate block finalization. [ozone]
Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on code in PR #6164:
URL: https://github.com/apache/ozone/pull/6164#discussion_r1483949570
##########
hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java:
##########
@@ -154,25 +157,41 @@ public boolean recoverLease(Path f) throws IOException {
}
// finalize the final block and get block length
- List<OmKeyLocationInfo> locationInfoList = keyInfo.getLatestVersionLocations().getLocationList();
+ List<OmKeyLocationInfo> locationInfoList = leaseKeyInfo.getKeyInfo().getLatestVersionLocations().getLocationList();
if (!locationInfoList.isEmpty()) {
OmKeyLocationInfo block = locationInfoList.get(locationInfoList.size() - 1);
try {
block.setLength(getAdapter().finalizeBlock(block));
} catch (Throwable e) {
- if (!forceRecovery) {
+ if (e instanceof StorageContainerException && (((StorageContainerException) e).getResult().equals(NO_SUCH_BLOCK)
+ || ((StorageContainerException) e).getResult().equals(CONTAINER_NOT_FOUND))
+ && !leaseKeyInfo.getIsKeyInfo() && locationInfoList.size() > 1) {
+ locationInfoList = leaseKeyInfo.getKeyInfo().getLatestVersionLocations().getLocationList().subList(0,
+ locationInfoList.size() - 1);
+ block = locationInfoList.get(locationInfoList.size() - 1);
Review Comment:
Yes code is duplicated across FileSystem classes, I will file a new Jira to remove these duplication.
--
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
Re: [PR] HDDS-10242. [hsync] Handle penultimate block finalization. [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang merged PR #6164:
URL: https://github.com/apache/ozone/pull/6164
--
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
Re: [PR] HDDS-10242. [hsync] Handle penultimate block finalization. [ozone]
Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on code in PR #6164:
URL: https://github.com/apache/ozone/pull/6164#discussion_r1483833363
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/LeaseKeyInfo.java:
##########
@@ -0,0 +1,41 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.helpers;
+
+/**
+ * This class represents LeaseKeyInfo
+ * isKeyInfo represents whether key is from openKeyTable or keyTable
+ * true corresponds to keyInfo is from keyTable.
Review Comment:
Done
--
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
Re: [PR] HDDS-10242. [hsync] Handle penultimate block finalization. [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #6164:
URL: https://github.com/apache/ozone/pull/6164#discussion_r1483993300
##########
hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java:
##########
@@ -154,25 +157,41 @@ public boolean recoverLease(Path f) throws IOException {
}
// finalize the final block and get block length
- List<OmKeyLocationInfo> locationInfoList = keyInfo.getLatestVersionLocations().getLocationList();
+ List<OmKeyLocationInfo> locationInfoList = leaseKeyInfo.getKeyInfo().getLatestVersionLocations().getLocationList();
if (!locationInfoList.isEmpty()) {
OmKeyLocationInfo block = locationInfoList.get(locationInfoList.size() - 1);
try {
block.setLength(getAdapter().finalizeBlock(block));
} catch (Throwable e) {
- if (!forceRecovery) {
+ if (e instanceof StorageContainerException && (((StorageContainerException) e).getResult().equals(NO_SUCH_BLOCK)
+ || ((StorageContainerException) e).getResult().equals(CONTAINER_NOT_FOUND))
+ && !leaseKeyInfo.getIsKeyInfo() && locationInfoList.size() > 1) {
+ locationInfoList = leaseKeyInfo.getKeyInfo().getLatestVersionLocations().getLocationList().subList(0,
+ locationInfoList.size() - 1);
+ block = locationInfoList.get(locationInfoList.size() - 1);
Review Comment:
uh ok i see.
--
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
Re: [PR] HDDS-10242. [hsync] Handle penultimate block finalization. [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #6164:
URL: https://github.com/apache/ozone/pull/6164#discussion_r1483936339
##########
hadoop-ozone/ozonefs-hadoop3/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java:
##########
@@ -154,25 +157,41 @@ public boolean recoverLease(Path f) throws IOException {
}
// finalize the final block and get block length
- List<OmKeyLocationInfo> locationInfoList = keyInfo.getLatestVersionLocations().getLocationList();
+ List<OmKeyLocationInfo> locationInfoList = leaseKeyInfo.getKeyInfo().getLatestVersionLocations().getLocationList();
if (!locationInfoList.isEmpty()) {
OmKeyLocationInfo block = locationInfoList.get(locationInfoList.size() - 1);
try {
block.setLength(getAdapter().finalizeBlock(block));
} catch (Throwable e) {
- if (!forceRecovery) {
+ if (e instanceof StorageContainerException && (((StorageContainerException) e).getResult().equals(NO_SUCH_BLOCK)
+ || ((StorageContainerException) e).getResult().equals(CONTAINER_NOT_FOUND))
+ && !leaseKeyInfo.getIsKeyInfo() && locationInfoList.size() > 1) {
+ locationInfoList = leaseKeyInfo.getKeyInfo().getLatestVersionLocations().getLocationList().subList(0,
+ locationInfoList.size() - 1);
+ block = locationInfoList.get(locationInfoList.size() - 1);
Review Comment:
is this the same as
```suggestion
block = locationInfoList.get(locationInfoList.size() - 2);
```
--
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
Re: [PR] HDDS-10242. [hsync] Handle penultimate block finalization. [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #6164:
URL: https://github.com/apache/ozone/pull/6164#issuecomment-1936921414
Merged. Thanks @ashishkumar50 for fixing the bug and @smengcl for the review.
--
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
Re: [PR] HDDS-10242. [hsync] Handle penultimate block finalization. [ozone]
Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 closed pull request #6164: HDDS-10242. [hsync] Handle penultimate block finalization.
URL: https://github.com/apache/ozone/pull/6164
--
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