You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "steveloughran (via GitHub)" <gi...@apache.org> on 2023/03/19 19:34:10 UTC
[GitHub] [hadoop] steveloughran opened a new pull request, #5494: HADOOP-18425. ABFS rename recovery
steveloughran opened a new pull request, #5494:
URL: https://github.com/apache/hadoop/pull/5494
### Description of PR
This is @sreeb-msft's PR of #5448 with
* my switch to turn this on/off
* more logging on recovery
The handling is now exclusively in the AbfsClient class.
### How was this patch tested?
abfs test in progress; azure cardiff.
### For code changes:
- [ = Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
- [ ] Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the `LICENSE`, `LICENSE-binary`, `NOTICE-binary` files?
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a diff in pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#discussion_r1142007419
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -613,39 +644,58 @@ private void incrementAbfsRenamePath() {
* Exceptions raised in the probe of the destination are swallowed,
* so that they do not interfere with the original rename failures.
* @param source source path
+ * @param sourceEtag etag of source file. may be null or empty
* @param op Rename request REST operation response with non-null HTTP response
* @param destination rename destination path
- * @param sourceEtag etag of source file. may be null or empty
* @param tracingContext Tracks identifiers for request header
+ * @param isDir is the source a file or directory
* @return true if the file was successfully copied
*/
public boolean renameIdempotencyCheckOp(
final String source,
final String sourceEtag,
final AbfsRestOperation op,
final String destination,
- TracingContext tracingContext) {
+ TracingContext tracingContext,
+ final boolean isDir) {
Preconditions.checkArgument(op.hasResult(), "Operations has null HTTP response");
- if ((op.isARetriedRequest())
- && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)
- && isNotEmpty(sourceEtag)) {
-
+ if (!(op.isARetriedRequest())
+ && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)) {
+ // not an error
+ return false;
+ }
+ LOG.debug("Source not found on retry of rename({}, {}) isDir {} etag {}",
+ source, destination, isDir, sourceEtag);
+ if (isDir) {
+ // directory recovery is not supported.
+ // log and fail.
+ LOG.info("rename directory {} to {} failed; unable to recover",
+ source, destination);
+ return false;
+ }
+ if (isNotEmpty(sourceEtag)) {
Review Comment:
yes, but I want to log slightly differently. though actually it is confusion as isDir is true if resilience is off. will cut
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] snvijaya commented on a diff in pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "snvijaya (via GitHub)" <gi...@apache.org>.
snvijaya commented on code in PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#discussion_r1141705797
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -519,11 +528,26 @@ public AbfsClientRenameResult renamePath(
final String destination,
final String continuation,
final TracingContext tracingContext,
- final String sourceEtag,
+ String sourceEtag,
boolean isMetadataIncompleteState)
throws AzureBlobFileSystemException {
final List<AbfsHttpHeader> requestHeaders = createDefaultHeaders();
+ // etag passed in, so source is a file
+ final boolean hasEtag = isEmpty(sourceEtag);
+ boolean isDir = !hasEtag;
+ if (!hasEtag && renameResilience) {
Review Comment:
sourceEtag should be passed down only for HNS account as eTag does not remain the same when its FNS (as rename = copy to destination and delete source).
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] saxenapranav commented on a diff in pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "saxenapranav (via GitHub)" <gi...@apache.org>.
saxenapranav commented on code in PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#discussion_r1141630233
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -105,6 +106,11 @@ public class AbfsClient implements Closeable {
private final ListeningScheduledExecutorService executorService;
+ /**
+ * Enable resilient rename.
+ */
+ private boolean renameResilience;
Review Comment:
nit: lets have it final.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -613,39 +644,58 @@ private void incrementAbfsRenamePath() {
* Exceptions raised in the probe of the destination are swallowed,
* so that they do not interfere with the original rename failures.
* @param source source path
+ * @param sourceEtag etag of source file. may be null or empty
* @param op Rename request REST operation response with non-null HTTP response
* @param destination rename destination path
- * @param sourceEtag etag of source file. may be null or empty
* @param tracingContext Tracks identifiers for request header
+ * @param isDir is the source a file or directory
* @return true if the file was successfully copied
*/
public boolean renameIdempotencyCheckOp(
final String source,
final String sourceEtag,
final AbfsRestOperation op,
final String destination,
- TracingContext tracingContext) {
+ TracingContext tracingContext,
+ final boolean isDir) {
Preconditions.checkArgument(op.hasResult(), "Operations has null HTTP response");
- if ((op.isARetriedRequest())
- && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)
- && isNotEmpty(sourceEtag)) {
-
+ if (!(op.isARetriedRequest())
+ && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)) {
+ // not an error
+ return false;
+ }
+ LOG.debug("Source not found on retry of rename({}, {}) isDir {} etag {}",
Review Comment:
this can come even if op was retried and statusCode is not equal to 404
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -613,39 +644,58 @@ private void incrementAbfsRenamePath() {
* Exceptions raised in the probe of the destination are swallowed,
* so that they do not interfere with the original rename failures.
* @param source source path
+ * @param sourceEtag etag of source file. may be null or empty
* @param op Rename request REST operation response with non-null HTTP response
* @param destination rename destination path
- * @param sourceEtag etag of source file. may be null or empty
* @param tracingContext Tracks identifiers for request header
+ * @param isDir is the source a file or directory
* @return true if the file was successfully copied
*/
public boolean renameIdempotencyCheckOp(
final String source,
final String sourceEtag,
final AbfsRestOperation op,
final String destination,
- TracingContext tracingContext) {
+ TracingContext tracingContext,
+ final boolean isDir) {
Preconditions.checkArgument(op.hasResult(), "Operations has null HTTP response");
- if ((op.isARetriedRequest())
- && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)
- && isNotEmpty(sourceEtag)) {
-
+ if (!(op.isARetriedRequest())
+ && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)) {
+ // not an error
+ return false;
+ }
+ LOG.debug("Source not found on retry of rename({}, {}) isDir {} etag {}",
+ source, destination, isDir, sourceEtag);
+ if (isDir) {
+ // directory recovery is not supported.
+ // log and fail.
+ LOG.info("rename directory {} to {} failed; unable to recover",
+ source, destination);
+ return false;
+ }
+ if (isNotEmpty(sourceEtag)) {
Review Comment:
As per code in renamePath(),
```
isDir = !isNotEmpty(sourceEtag)
```
Should we use the same relation in this method to reduce confusion, instead of having a new argument `isDir`.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -613,39 +644,58 @@ private void incrementAbfsRenamePath() {
* Exceptions raised in the probe of the destination are swallowed,
* so that they do not interfere with the original rename failures.
* @param source source path
+ * @param sourceEtag etag of source file. may be null or empty
* @param op Rename request REST operation response with non-null HTTP response
* @param destination rename destination path
- * @param sourceEtag etag of source file. may be null or empty
* @param tracingContext Tracks identifiers for request header
+ * @param isDir is the source a file or directory
* @return true if the file was successfully copied
*/
public boolean renameIdempotencyCheckOp(
final String source,
final String sourceEtag,
final AbfsRestOperation op,
final String destination,
- TracingContext tracingContext) {
+ TracingContext tracingContext,
+ final boolean isDir) {
Preconditions.checkArgument(op.hasResult(), "Operations has null HTTP response");
- if ((op.isARetriedRequest())
- && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)
- && isNotEmpty(sourceEtag)) {
-
+ if (!(op.isARetriedRequest())
+ && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)) {
+ // not an error
+ return false;
+ }
+ LOG.debug("Source not found on retry of rename({}, {}) isDir {} etag {}",
+ source, destination, isDir, sourceEtag);
+ if (isDir) {
+ // directory recovery is not supported.
+ // log and fail.
+ LOG.info("rename directory {} to {} failed; unable to recover",
+ source, destination);
+ return false;
+ }
Review Comment:
this becomes equavalent to:
```
op.isARetriedRequest() || op.getResult().getStatusCode != HttpURLConnection.HTTP_NOT_FOUND
```
The older condition:
```
if ((op.isARetriedRequest())
&& (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)
```
is changed.
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsRenameRetryRecovery.java:
##########
@@ -123,6 +138,112 @@ public void testRenameFailuresDueToIncompleteMetadata() throws Exception {
}
+ AbfsClient getMockAbfsClient() throws IOException {
+ AzureBlobFileSystem fs = getFileSystem();
+
+ // specifying AbfsHttpOperation mock behavior
+
+ // mock object representing the 404 path not found result
+ AbfsHttpOperation mockHttp404Op = Mockito.mock(AbfsHttpOperation.class);
Review Comment:
It would be awesome if we can use server integration in picture. Have added a comment on @sreeb-msft 's pr https://github.com/apache/hadoop/pull/5488#discussion_r1141679607. Suggested changes in https://github.com/saxenapranav/hadoop/commit/5247e12bf2b96f8397d4c6df0865117a18dd13fd
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java:
##########
@@ -46,6 +46,9 @@
import javax.annotation.Nullable;
import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.fs.EtagSource;
+import org.apache.hadoop.fs.azurebfs.services.AbfsHttpOperation;
+import org.apache.hadoop.fs.azurebfs.services.AbfsRestOperation;
Review Comment:
imports not required.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a diff in pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#discussion_r1141965748
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -613,39 +644,58 @@ private void incrementAbfsRenamePath() {
* Exceptions raised in the probe of the destination are swallowed,
* so that they do not interfere with the original rename failures.
* @param source source path
+ * @param sourceEtag etag of source file. may be null or empty
* @param op Rename request REST operation response with non-null HTTP response
* @param destination rename destination path
- * @param sourceEtag etag of source file. may be null or empty
* @param tracingContext Tracks identifiers for request header
+ * @param isDir is the source a file or directory
* @return true if the file was successfully copied
*/
public boolean renameIdempotencyCheckOp(
final String source,
final String sourceEtag,
final AbfsRestOperation op,
final String destination,
- TracingContext tracingContext) {
+ TracingContext tracingContext,
+ final boolean isDir) {
Preconditions.checkArgument(op.hasResult(), "Operations has null HTTP response");
- if ((op.isARetriedRequest())
- && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)
- && isNotEmpty(sourceEtag)) {
-
+ if (!(op.isARetriedRequest())
+ && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)) {
+ // not an error
+ return false;
+ }
+ LOG.debug("Source not found on retry of rename({}, {}) isDir {} etag {}",
+ source, destination, isDir, sourceEtag);
+ if (isDir) {
+ // directory recovery is not supported.
+ // log and fail.
+ LOG.info("rename directory {} to {} failed; unable to recover",
+ source, destination);
+ return false;
+ }
+ if (isNotEmpty(sourceEtag)) {
Review Comment:
ok. I do want this to work even when resilience is off, but passing in "" will do that, won't it?
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a diff in pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#discussion_r1141966410
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -519,11 +528,26 @@ public AbfsClientRenameResult renamePath(
final String destination,
final String continuation,
final TracingContext tracingContext,
- final String sourceEtag,
+ String sourceEtag,
boolean isMetadataIncompleteState)
throws AzureBlobFileSystemException {
final List<AbfsHttpHeader> requestHeaders = createDefaultHeaders();
+ // etag passed in, so source is a file
+ final boolean hasEtag = isEmpty(sourceEtag);
Review Comment:
ooh, nice catch. scarily nice. what happens when i code on a weekend
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a diff in pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#discussion_r1141957438
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -613,39 +644,58 @@ private void incrementAbfsRenamePath() {
* Exceptions raised in the probe of the destination are swallowed,
* so that they do not interfere with the original rename failures.
* @param source source path
+ * @param sourceEtag etag of source file. may be null or empty
* @param op Rename request REST operation response with non-null HTTP response
* @param destination rename destination path
- * @param sourceEtag etag of source file. may be null or empty
* @param tracingContext Tracks identifiers for request header
+ * @param isDir is the source a file or directory
* @return true if the file was successfully copied
*/
public boolean renameIdempotencyCheckOp(
final String source,
final String sourceEtag,
final AbfsRestOperation op,
final String destination,
- TracingContext tracingContext) {
+ TracingContext tracingContext,
+ final boolean isDir) {
Preconditions.checkArgument(op.hasResult(), "Operations has null HTTP response");
- if ((op.isARetriedRequest())
- && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)
- && isNotEmpty(sourceEtag)) {
-
+ if (!(op.isARetriedRequest())
+ && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)) {
+ // not an error
+ return false;
+ }
+ LOG.debug("Source not found on retry of rename({}, {}) isDir {} etag {}",
Review Comment:
the existing code only handles 404. Are you saying there are other errors we should look for, such as some 500+ value? if so: which ones
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#issuecomment-1477033452
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 36s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 2 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 15m 50s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 25m 51s | | trunk passed |
| +1 :green_heart: | compile | 23m 0s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 20m 33s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 3m 46s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 39s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 0s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 30s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 3m 55s | | trunk passed |
| +1 :green_heart: | shadedclient | 20m 38s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 27s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 29s | | the patch passed |
| +1 :green_heart: | compile | 22m 32s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| -1 :x: | javac | 22m 32s | [/results-compile-javac-root-jdkUbuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5494/2/artifact/out/results-compile-javac-root-jdkUbuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1.txt) | root-jdkUbuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 generated 2 new + 2825 unchanged - 0 fixed = 2827 total (was 2825) |
| +1 :green_heart: | compile | 20m 32s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| -1 :x: | javac | 20m 32s | [/results-compile-javac-root-jdkPrivateBuild-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5494/2/artifact/out/results-compile-javac-root-jdkPrivateBuild-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09.txt) | root-jdkPrivateBuild-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 generated 2 new + 2622 unchanged - 0 fixed = 2624 total (was 2622) |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 3m 38s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5494/2/artifact/out/results-checkstyle-root.txt) | root: The patch generated 6 new + 29 unchanged - 1 fixed = 35 total (was 30) |
| +1 :green_heart: | mvnsite | 2m 37s | | the patch passed |
| +1 :green_heart: | javadoc | 1m 54s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 36s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 5s | | the patch passed |
| +1 :green_heart: | shadedclient | 20m 59s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 16s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 2m 21s | | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 1m 1s | | The patch does not generate ASF License warnings. |
| | | 225m 55s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5494/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5494 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 1819407248d6 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / e36b4c2c2a8b2de2aefd3b94f27a4ac6694015ca |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5494/2/testReport/ |
| Max. process+thread count | 2289 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-azure U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5494/2/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#issuecomment-1475435086
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 45s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 15m 41s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 28m 41s | | trunk passed |
| +1 :green_heart: | compile | 25m 4s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 21m 35s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 3m 59s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 26s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 46s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 18s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 3m 55s | | trunk passed |
| +1 :green_heart: | shadedclient | 23m 34s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 23s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 34s | | the patch passed |
| +1 :green_heart: | compile | 24m 29s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| -1 :x: | javac | 24m 29s | [/results-compile-javac-root-jdkUbuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5494/1/artifact/out/results-compile-javac-root-jdkUbuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1.txt) | root-jdkUbuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 generated 2 new + 2825 unchanged - 0 fixed = 2827 total (was 2825) |
| +1 :green_heart: | compile | 21m 36s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| -1 :x: | javac | 21m 36s | [/results-compile-javac-root-jdkPrivateBuild-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5494/1/artifact/out/results-compile-javac-root-jdkPrivateBuild-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09.txt) | root-jdkPrivateBuild-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 generated 2 new + 2622 unchanged - 0 fixed = 2624 total (was 2622) |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 3m 48s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5494/1/artifact/out/results-checkstyle-root.txt) | root: The patch generated 8 new + 28 unchanged - 0 fixed = 36 total (was 28) |
| +1 :green_heart: | mvnsite | 2m 25s | | the patch passed |
| +1 :green_heart: | javadoc | 1m 35s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 18s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 2s | | the patch passed |
| +1 :green_heart: | shadedclient | 23m 55s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 18s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 2m 9s | | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 0m 51s | | The patch does not generate ASF License warnings. |
| | | 238m 24s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5494/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5494 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux bbc482f5a226 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 70a8d0fb3956b2a0e2c343373475ef7a0e75ab08 |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5494/1/testReport/ |
| Max. process+thread count | 3137 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-azure U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5494/1/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a diff in pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#discussion_r1141947058
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -613,39 +644,58 @@ private void incrementAbfsRenamePath() {
* Exceptions raised in the probe of the destination are swallowed,
* so that they do not interfere with the original rename failures.
* @param source source path
+ * @param sourceEtag etag of source file. may be null or empty
* @param op Rename request REST operation response with non-null HTTP response
* @param destination rename destination path
- * @param sourceEtag etag of source file. may be null or empty
* @param tracingContext Tracks identifiers for request header
+ * @param isDir is the source a file or directory
* @return true if the file was successfully copied
*/
public boolean renameIdempotencyCheckOp(
final String source,
final String sourceEtag,
final AbfsRestOperation op,
final String destination,
- TracingContext tracingContext) {
+ TracingContext tracingContext,
+ final boolean isDir) {
Preconditions.checkArgument(op.hasResult(), "Operations has null HTTP response");
- if ((op.isARetriedRequest())
- && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)
- && isNotEmpty(sourceEtag)) {
-
+ if (!(op.isARetriedRequest())
+ && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)) {
+ // not an error
+ return false;
+ }
+ LOG.debug("Source not found on retry of rename({}, {}) isDir {} etag {}",
+ source, destination, isDir, sourceEtag);
+ if (isDir) {
+ // directory recovery is not supported.
+ // log and fail.
+ LOG.info("rename directory {} to {} failed; unable to recover",
+ source, destination);
+ return false;
+ }
+ if (isNotEmpty(sourceEtag)) {
// Server has returned HTTP 404, which means rename source no longer
// exists. Check on destination status and if its etag matches
// that of the source, consider it to be a success.
- LOG.debug("rename {} to {} failed, checking etag of destination",
+ // the source tag was either passed in from a manifest commit or
+ // retrieved when rename recovery is enabled.
+ LOG.info("rename {} to {} failed, checking etag of destination",
source, destination);
try {
final AbfsRestOperation destStatusOp = getPathStatus(destination,
false, tracingContext);
final AbfsHttpOperation result = destStatusOp.getResult();
- return result.getStatusCode() == HttpURLConnection.HTTP_OK
+ final boolean recovered = result.getStatusCode() == HttpURLConnection.HTTP_OK
&& sourceEtag.equals(extractEtagHeader(result));
- } catch (AzureBlobFileSystemException ignored) {
+ LOG.info("File rename has taken place: recovery completed");
Review Comment:
fixed
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a diff in pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#discussion_r1141948963
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -613,39 +644,58 @@ private void incrementAbfsRenamePath() {
* Exceptions raised in the probe of the destination are swallowed,
* so that they do not interfere with the original rename failures.
* @param source source path
+ * @param sourceEtag etag of source file. may be null or empty
* @param op Rename request REST operation response with non-null HTTP response
* @param destination rename destination path
- * @param sourceEtag etag of source file. may be null or empty
* @param tracingContext Tracks identifiers for request header
+ * @param isDir is the source a file or directory
* @return true if the file was successfully copied
*/
public boolean renameIdempotencyCheckOp(
final String source,
final String sourceEtag,
final AbfsRestOperation op,
final String destination,
- TracingContext tracingContext) {
+ TracingContext tracingContext,
+ final boolean isDir) {
Preconditions.checkArgument(op.hasResult(), "Operations has null HTTP response");
- if ((op.isARetriedRequest())
- && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)
- && isNotEmpty(sourceEtag)) {
-
+ if (!(op.isARetriedRequest())
+ && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)) {
+ // not an error
+ return false;
+ }
+ LOG.debug("Source not found on retry of rename({}, {}) isDir {} etag {}",
+ source, destination, isDir, sourceEtag);
+ if (isDir) {
+ // directory recovery is not supported.
+ // log and fail.
+ LOG.info("rename directory {} to {} failed; unable to recover",
+ source, destination);
+ return false;
+ }
+ if (isNotEmpty(sourceEtag)) {
// Server has returned HTTP 404, which means rename source no longer
// exists. Check on destination status and if its etag matches
// that of the source, consider it to be a success.
Review Comment:
will cut back
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a diff in pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#discussion_r1141960454
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -613,39 +644,58 @@ private void incrementAbfsRenamePath() {
* Exceptions raised in the probe of the destination are swallowed,
* so that they do not interfere with the original rename failures.
* @param source source path
+ * @param sourceEtag etag of source file. may be null or empty
* @param op Rename request REST operation response with non-null HTTP response
* @param destination rename destination path
- * @param sourceEtag etag of source file. may be null or empty
* @param tracingContext Tracks identifiers for request header
+ * @param isDir is the source a file or directory
* @return true if the file was successfully copied
*/
public boolean renameIdempotencyCheckOp(
final String source,
final String sourceEtag,
final AbfsRestOperation op,
final String destination,
- TracingContext tracingContext) {
+ TracingContext tracingContext,
+ final boolean isDir) {
Preconditions.checkArgument(op.hasResult(), "Operations has null HTTP response");
- if ((op.isARetriedRequest())
- && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)
- && isNotEmpty(sourceEtag)) {
-
+ if (!(op.isARetriedRequest())
+ && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)) {
+ // not an error
+ return false;
+ }
+ LOG.debug("Source not found on retry of rename({}, {}) isDir {} etag {}",
+ source, destination, isDir, sourceEtag);
+ if (isDir) {
+ // directory recovery is not supported.
+ // log and fail.
+ LOG.info("rename directory {} to {} failed; unable to recover",
+ source, destination);
+ return false;
+ }
Review Comment:
ok, let me copy and paste again. i think i was trying to do the inversion logic but may have stopped partway through.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] saxenapranav commented on a diff in pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "saxenapranav (via GitHub)" <gi...@apache.org>.
saxenapranav commented on code in PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#discussion_r1141832660
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -519,11 +528,26 @@ public AbfsClientRenameResult renamePath(
final String destination,
final String continuation,
final TracingContext tracingContext,
- final String sourceEtag,
+ String sourceEtag,
boolean isMetadataIncompleteState)
throws AzureBlobFileSystemException {
final List<AbfsHttpHeader> requestHeaders = createDefaultHeaders();
+ // etag passed in, so source is a file
+ final boolean hasEtag = isEmpty(sourceEtag);
Review Comment:
should it be
```
hasEtag = !isEmpty(sourceEtag)
```
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran closed pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran closed pull request #5494: HADOOP-18425. ABFS rename recovery
URL: https://github.com/apache/hadoop/pull/5494
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a diff in pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#discussion_r1141953901
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -519,11 +528,26 @@ public AbfsClientRenameResult renamePath(
final String destination,
final String continuation,
final TracingContext tracingContext,
- final String sourceEtag,
+ String sourceEtag,
boolean isMetadataIncompleteState)
throws AzureBlobFileSystemException {
final List<AbfsHttpHeader> requestHeaders = createDefaultHeaders();
+ // etag passed in, so source is a file
+ final boolean hasEtag = isEmpty(sourceEtag);
+ boolean isDir = !hasEtag;
+ if (!hasEtag && renameResilience) {
Review Comment:
yeah, but AFAIK there's no way to check in the client whether it is available.
it can only get passed in through the manifest committer, and as `ABFS.createResilientCommitSupport()` requires etag preservation, it won't be doing it on a non-HNS store.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#issuecomment-1475407372
reviews encouraged from people, especially @sreeb-msft @snvijaya @saxenapranav @hiteshs @mukund-thakur @mehakmeet @taklwu
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] saxenapranav commented on a diff in pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "saxenapranav (via GitHub)" <gi...@apache.org>.
saxenapranav commented on code in PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#discussion_r1142106366
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -613,39 +644,58 @@ private void incrementAbfsRenamePath() {
* Exceptions raised in the probe of the destination are swallowed,
* so that they do not interfere with the original rename failures.
* @param source source path
+ * @param sourceEtag etag of source file. may be null or empty
* @param op Rename request REST operation response with non-null HTTP response
* @param destination rename destination path
- * @param sourceEtag etag of source file. may be null or empty
* @param tracingContext Tracks identifiers for request header
+ * @param isDir is the source a file or directory
* @return true if the file was successfully copied
*/
public boolean renameIdempotencyCheckOp(
final String source,
final String sourceEtag,
final AbfsRestOperation op,
final String destination,
- TracingContext tracingContext) {
+ TracingContext tracingContext,
+ final boolean isDir) {
Preconditions.checkArgument(op.hasResult(), "Operations has null HTTP response");
- if ((op.isARetriedRequest())
- && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)
- && isNotEmpty(sourceEtag)) {
-
+ if (!(op.isARetriedRequest())
+ && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)) {
+ // not an error
+ return false;
+ }
+ LOG.debug("Source not found on retry of rename({}, {}) isDir {} etag {}",
Review Comment:
this function gets invoked for any AbfsBlobFileSystemException whether it be because of 404 or any other: https://github.com/steveloughran/hadoop/blob/70a8d0fb3956b2a0e2c343373475ef7a0e75ab08/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java#L607.
now because at line 668, it is because :
```
op.isARetriedRequest() || op.getResult().getStatusCode() != HttpUrlConnection.HTTP_NOT_FOUND
```
any retried operation will come on this line.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#issuecomment-1477179763
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 46s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 2 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 15m 33s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 28m 57s | | trunk passed |
| +1 :green_heart: | compile | 25m 2s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | compile | 21m 42s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | checkstyle | 4m 3s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 24s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 44s | | trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 19s | | trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 3m 57s | | trunk passed |
| +1 :green_heart: | shadedclient | 23m 52s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 49s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 37s | | the patch passed |
| +1 :green_heart: | compile | 24m 30s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| -1 :x: | javac | 24m 30s | [/results-compile-javac-root-jdkUbuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5494/3/artifact/out/results-compile-javac-root-jdkUbuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1.txt) | root-jdkUbuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 generated 2 new + 2825 unchanged - 0 fixed = 2827 total (was 2825) |
| +1 :green_heart: | compile | 21m 32s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| -1 :x: | javac | 21m 32s | [/results-compile-javac-root-jdkPrivateBuild-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5494/3/artifact/out/results-compile-javac-root-jdkPrivateBuild-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09.txt) | root-jdkPrivateBuild-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 generated 2 new + 2622 unchanged - 0 fixed = 2624 total (was 2622) |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 3m 55s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5494/3/artifact/out/results-checkstyle-root.txt) | root: The patch generated 6 new + 29 unchanged - 1 fixed = 35 total (was 30) |
| +1 :green_heart: | mvnsite | 2m 24s | | the patch passed |
| +1 :green_heart: | javadoc | 1m 37s | | the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 |
| +1 :green_heart: | javadoc | 1m 18s | | the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| +1 :green_heart: | spotbugs | 4m 3s | | the patch passed |
| +1 :green_heart: | shadedclient | 24m 23s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 19m 14s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 2m 10s | | hadoop-azure in the patch passed. |
| +1 :green_heart: | asflicense | 0m 50s | | The patch does not generate ASF License warnings. |
| | | 240m 59s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5494/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/5494 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 7b374dbc308e 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / a303e33c02616db4c1b165c1797b60cad5801597 |
| Default Java | Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5494/3/testReport/ |
| Max. process+thread count | 3137 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-azure U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5494/3/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] mehakmeet commented on a diff in pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "mehakmeet (via GitHub)" <gi...@apache.org>.
mehakmeet commented on code in PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#discussion_r1142877888
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -38,6 +38,7 @@
import java.util.concurrent.TimeUnit;
import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException;
Review Comment:
import seem to be in the wrong place here.
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsRenameRetryRecovery.java:
##########
@@ -123,6 +143,257 @@ public void testRenameFailuresDueToIncompleteMetadata() throws Exception {
}
+ AbfsClient getMockAbfsClient() throws IOException {
+ AzureBlobFileSystem fs = getFileSystem();
+
+ // adding mock objects to current AbfsClient
+ AbfsClient spyClient = Mockito.spy(fs.getAbfsStore().getClient());
+
+ Mockito.doAnswer(answer -> {
+ AbfsRestOperation op = new AbfsRestOperation(AbfsRestOperationType.RenamePath,
+ spyClient, HTTP_METHOD_PUT, answer.getArgument(0), answer.getArgument(1));
+ AbfsRestOperation spiedOp = Mockito.spy(op);
+ addSpyBehavior(spiedOp, op, spyClient);
+ return spiedOp;
+ }).when(spyClient).createRenameRestOperation(nullable(URL.class), nullable(List.class));
+
+ return spyClient;
+
+ }
+
+ /**
+ * Spies on a rest operation to inject transient failure.
+ * the first createHttpOperation() invocation will return an abfs rest operation
+ * which will fail.
+ * @param spiedRestOp spied operation whose createHttpOperation() will fail first time
+ * @param normalRestOp normal operation the good operation
+ * @param client client.
+ * @throws IOException failure
+ */
+ private void addSpyBehavior(final AbfsRestOperation spiedRestOp,
+ final AbfsRestOperation normalRestOp,
+ final AbfsClient client)
+ throws IOException {
+ AbfsHttpOperation failingOperation = Mockito.spy(normalRestOp.createHttpOperation());
+ AbfsHttpOperation normalOp1 = normalRestOp.createHttpOperation();
+ executeThenFail(client, normalRestOp, failingOperation, normalOp1);
+ AbfsHttpOperation normalOp2 = normalRestOp.createHttpOperation();
+ normalOp2.getConnection().setRequestProperty(HttpHeaderConfigurations.AUTHORIZATION,
+ client.getAccessToken());
+
+ when(spiedRestOp.createHttpOperation())
+ .thenReturn(failingOperation)
+ .thenReturn(normalOp2);
+ }
+
+ /**
+ * Mock an idempotency failure by executing the normal operation, then
+ * raising an IOE.
+ * @param normalRestOp the rest operation used to sign the requests.
+ * @param failingOperation failing operation
+ * @param normalOp good operation
+ * @throws IOException failure
+ */
+ private void executeThenFail(final AbfsClient client,
+ final AbfsRestOperation normalRestOp,
+ final AbfsHttpOperation failingOperation,
+ final AbfsHttpOperation normalOp)
+ throws IOException {
+ Mockito.doAnswer(answer -> {
+ LOG.info("Executing first attempt with post-operation fault injection");
+ final byte[] buffer = answer.getArgument(0);
+ final int offset = answer.getArgument(1);
+ final int length = answer.getArgument(2);
+ normalRestOp.signRequest(normalOp, length);
+ normalOp.sendRequest(buffer, offset, length);
+ normalOp.processResponse(buffer, offset, length);
+ LOG.info("Actual outcome is {} \"{}\" \"{}\"; injecting failure",
+ normalOp.getStatusCode(),
+ normalOp.getStorageErrorCode(),
+ normalOp.getStorageErrorMessage());
+ throw new SocketException("connection-reset");
+ }).when(failingOperation).sendRequest(Mockito.nullable(byte[].class),
+ Mockito.nullable(int.class), Mockito.nullable(int.class));
+ }
+
+ /**
+ * This is the good outcome: resilient rename.
+ */
+ @Test
+ public void testRenameRecoverySrcDestEtagSame() throws IOException {
+ AzureBlobFileSystem fs = getFileSystem();
+ TracingContext testTracingContext = getTestTracingContext(fs, false);
+
+ Assume.assumeTrue(fs.getAbfsStore().getIsNamespaceEnabled(testTracingContext));
+
+ AbfsClient mockClient = getMockAbfsClient();
+
+ String base = "/" + getMethodName();
+ String path1 = base + "/dummyFile1";
+ String path2 = base + "/dummyFile2";
+
+ touch(new Path(path1));
+
+ // 404 and retry, send sourceEtag as null
+ // source eTag matches -> rename should pass even when execute throws exception
+ final AbfsClientRenameResult result =
+ mockClient.renamePath(path1, path2, null, testTracingContext, null, false);
+ Assertions.assertThat(result.isRenameRecovered())
+ .describedAs("rename result recovered flag of %s", result)
+ .isTrue();
+ }
+
+ /**
+ * execute a failing rename but have the file at the far end not match.
+ * This is done by explicitly passing in a made up etag for the source
+ * etag and creating a file at the far end.
+ * The first rename will actually fail with a path exists exception,
+ * but as that is swallowed, it's not a problem.
+ */
+ @Test
+ public void testRenameRecoverySourceDestEtagDifferent() throws Exception {
+ AzureBlobFileSystem fs = getFileSystem();
+ TracingContext testTracingContext = getTestTracingContext(fs, false);
+
+ Assume.assumeTrue(fs.getAbfsStore().getIsNamespaceEnabled(testTracingContext));
+
+ AbfsClient spyClient = getMockAbfsClient();
+
+ String base = "/" + getMethodName();
+ String path1 = base + "/dummyFile1";
+ String path2 = base + "/dummyFile2";
+
+ touch(new Path(path2));
+
+ // source eTag does not match -> throw exception
+ expectErrorCode(SOURCE_PATH_NOT_FOUND, intercept(AbfsRestOperationException.class, () ->
+ spyClient.renamePath(path1, path2, null, testTracingContext, "source", false))
+ );
+ }
+
+ /**
+ * Assert that an exception failed with a specific error code.
+ * @param code code
+ * @param e exception
+ * @throws AbfsRestOperationException if there is a mismatch
+ */
+ private static void expectErrorCode(final AzureServiceErrorCode code,
+ final AbfsRestOperationException e) throws AbfsRestOperationException {
+ if (e.getErrorCode() != code) {
+ throw e;
+ }
+ }
+
+ /**
+ * Directory rename failure is unrecoverable.
+ */
+ @Test
+ public void testDirRenameRecoveryUnsupported() throws Exception {
+ AzureBlobFileSystem fs = getFileSystem();
+ TracingContext testTracingContext = getTestTracingContext(fs, false);
+
+ Assume.assumeTrue(fs.getAbfsStore().getIsNamespaceEnabled(testTracingContext));
+
+ AbfsClient spyClient = getMockAbfsClient();
+
+ String base = "/" + getMethodName();
+ String path1 = base + "/dummyDir1";
+ String path2 = base + "/dummyDir2";
+
+ fs.mkdirs(new Path(path1));
+
+ // source eTag does not match -> throw exception
+ expectErrorCode(SOURCE_PATH_NOT_FOUND, intercept(AbfsRestOperationException.class, () ->
+ spyClient.renamePath(path1, path2, null, testTracingContext, null, false)));
+ }
+
+ /**
+ * Even with failures, having
Review Comment:
incomplete javadocs?
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -157,6 +166,9 @@ private AbfsClient(final URL baseUrl, final SharedKeyCredentials sharedKeyCreden
new ThreadFactoryBuilder().setNameFormat("AbfsClient Lease Ops").setDaemon(true).build();
this.executorService = MoreExecutors.listeningDecorator(
HadoopExecutors.newScheduledThreadPool(this.abfsConfiguration.getNumLeaseThreads(), tf));
+ // rename resilience
+ renameResilience = abfsConfiguration.getRenameResilience();
+ LOG.debug("Rename resilience is {}",renameResilience);
Review Comment:
nit: space after ",".
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#issuecomment-1476907289
so hdfs returns false if source not found, or dest exists
```xml
<property>
<name>fs.contract.rename-returns-false-if-dest-exists</name>
<value>true</value>
</property>
<property>
<name>fs.contract.rename-returns-false-if-source-missing</name>
<value>true</value>
</property>
```
abfs does the same.
s3a client blows up with meaningful errors
```xml
<property>
<name>fs.contract.rename-returns-false-if-source-missing</name>
<value>false</value>
</property>
<property>
<name>fs.contract.rename-returns-false-if-dest-exists</name>
<value>false</value>
</property>
```
I'm more in favour of the "blow up" strategy; if you look at almost all uses of rename it is code like:
```java
if (!rename(src, dest)) throw new Exception("rename failed we don't know why")p
```
and nobody ever seems to complain about the s3a failure...but then of course rename is broken there for other reasons, so maybe code just avoids it (e.g. committers).
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a diff in pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#discussion_r1141944816
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FilterFileSystem.java:
##########
@@ -76,6 +76,8 @@ public FilterFileSystem(FileSystem fs) {
this.statistics = fs.statistics;
}
+
Review Comment:
accidental edit; will revert
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#issuecomment-1475397295
tests good except for timeout in all the lease runs
```
O]
[ERROR] Errors:
[ERROR] ITestAzureBlobFileSystemLease.testInfiniteLease:284 » TestTimedOut test timed ...
[ERROR] ITestAzureBlobFileSystemLease.testTwoWritersCreateAppendNoInfiniteLease:177->twoWriters:166 » TestTimedOut
[ERROR] ITestAzureBlobFileSystemLease.testTwoWritersCreateAppendWithInfiniteLeaseEnabled:186->twoWriters:154 » TestTimedOut
[ERROR] ITestAzureBlobFileSystemLease.testWriteAfterBreakLease:223->lambda$testWriteAfterBreakLease$2:225 » TestTimedOut
[INFO]
```
not sure what is up there. I had been playing with leases recently but my site settings haven't set any
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#issuecomment-1475398833
and yes, a test run on its own works
```
NFO] Running org.apache.hadoop.fs.azurebfs.ITestAzureBlobFileSystemLease
[INFO] Tests run: 13, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 84.74 s - in org.apache.hadoop.fs.azurebfs.ITestAzureBlobFileSystemLease
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 13, Failures: 0, Errors: 0, Skipped: 0
```
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#issuecomment-1476895301
running the itests with the new pr and it is interesting because: when resilience is enabled, ITestAbfsFileSystemContractRename fail because of different outcomes from what is defined in the contract XML file.
that is actually what is expected according to https://hadoop.apache.org/docs/stable/hadoop-project-dist/hadoop-common/filesystem/filesystem.html#boolean_rename.28Path_src.2C_Path_d.29, so I'm not sure why it doesn't happen today. will investigate
```
java.lang.AssertionError: Renaming a missing file unexpectedly threw an exception
at org.apache.hadoop.fs.contract.ContractTestUtils.fail(ContractTestUtils.java:548)
at org.apache.hadoop.fs.contract.AbstractContractRenameTest.testRenameNonexistentFile(AbstractContractRenameTest.java:77)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:61)
at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:299)
at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:293)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at java.lang.Thread.run(Thread.java:750)
Caused by: java.io.FileNotFoundException: Operation failed: "The specified path does not exist.", 404, HEAD, https://stevelukwest.dfs.core.windows.net/stevel-testing/test/testRenameNonexistentFileSrc?upn=false&action=getStatus&timeout=90
at org.apache.hadoop.fs.azurebfs.AzureBlobFileSystem.checkException(AzureBlobFileSystem.java:1481)
at org.apache.hadoop.fs.azurebfs.AzureBlobFileSystem.rename(AzureBlobFileSystem.java:466)
at org.apache.hadoop.fs.contract.AbstractFSContractTestBase.rename(AbstractFSContractTestBase.java:388)
at org.apache.hadoop.fs.contract.AbstractContractRenameTest.testRenameNonexistentFile(AbstractContractRenameTest.java:62)
... 15 more
```
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] mukund-thakur commented on a diff in pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "mukund-thakur (via GitHub)" <gi...@apache.org>.
mukund-thakur commented on code in PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#discussion_r1141673602
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FilterFileSystem.java:
##########
@@ -76,6 +76,8 @@ public FilterFileSystem(FileSystem fs) {
this.statistics = fs.statistics;
}
+
Review Comment:
cut extra lines?
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -613,39 +644,58 @@ private void incrementAbfsRenamePath() {
* Exceptions raised in the probe of the destination are swallowed,
* so that they do not interfere with the original rename failures.
* @param source source path
+ * @param sourceEtag etag of source file. may be null or empty
* @param op Rename request REST operation response with non-null HTTP response
* @param destination rename destination path
- * @param sourceEtag etag of source file. may be null or empty
* @param tracingContext Tracks identifiers for request header
+ * @param isDir is the source a file or directory
* @return true if the file was successfully copied
*/
public boolean renameIdempotencyCheckOp(
final String source,
final String sourceEtag,
final AbfsRestOperation op,
final String destination,
- TracingContext tracingContext) {
+ TracingContext tracingContext,
+ final boolean isDir) {
Preconditions.checkArgument(op.hasResult(), "Operations has null HTTP response");
- if ((op.isARetriedRequest())
- && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)
- && isNotEmpty(sourceEtag)) {
-
+ if (!(op.isARetriedRequest())
+ && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)) {
+ // not an error
+ return false;
+ }
+ LOG.debug("Source not found on retry of rename({}, {}) isDir {} etag {}",
+ source, destination, isDir, sourceEtag);
+ if (isDir) {
+ // directory recovery is not supported.
+ // log and fail.
+ LOG.info("rename directory {} to {} failed; unable to recover",
+ source, destination);
+ return false;
+ }
+ if (isNotEmpty(sourceEtag)) {
// Server has returned HTTP 404, which means rename source no longer
// exists. Check on destination status and if its etag matches
// that of the source, consider it to be a success.
Review Comment:
these comments not valid here.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -613,39 +644,58 @@ private void incrementAbfsRenamePath() {
* Exceptions raised in the probe of the destination are swallowed,
* so that they do not interfere with the original rename failures.
* @param source source path
+ * @param sourceEtag etag of source file. may be null or empty
* @param op Rename request REST operation response with non-null HTTP response
* @param destination rename destination path
- * @param sourceEtag etag of source file. may be null or empty
* @param tracingContext Tracks identifiers for request header
+ * @param isDir is the source a file or directory
* @return true if the file was successfully copied
*/
public boolean renameIdempotencyCheckOp(
final String source,
final String sourceEtag,
final AbfsRestOperation op,
final String destination,
- TracingContext tracingContext) {
+ TracingContext tracingContext,
+ final boolean isDir) {
Preconditions.checkArgument(op.hasResult(), "Operations has null HTTP response");
- if ((op.isARetriedRequest())
- && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)
- && isNotEmpty(sourceEtag)) {
-
+ if (!(op.isARetriedRequest())
Review Comment:
add a comment here.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -613,39 +644,58 @@ private void incrementAbfsRenamePath() {
* Exceptions raised in the probe of the destination are swallowed,
* so that they do not interfere with the original rename failures.
* @param source source path
+ * @param sourceEtag etag of source file. may be null or empty
* @param op Rename request REST operation response with non-null HTTP response
* @param destination rename destination path
- * @param sourceEtag etag of source file. may be null or empty
* @param tracingContext Tracks identifiers for request header
+ * @param isDir is the source a file or directory
* @return true if the file was successfully copied
*/
public boolean renameIdempotencyCheckOp(
final String source,
final String sourceEtag,
final AbfsRestOperation op,
final String destination,
- TracingContext tracingContext) {
+ TracingContext tracingContext,
+ final boolean isDir) {
Preconditions.checkArgument(op.hasResult(), "Operations has null HTTP response");
- if ((op.isARetriedRequest())
- && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)
- && isNotEmpty(sourceEtag)) {
-
+ if (!(op.isARetriedRequest())
+ && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)) {
+ // not an error
+ return false;
+ }
+ LOG.debug("Source not found on retry of rename({}, {}) isDir {} etag {}",
+ source, destination, isDir, sourceEtag);
+ if (isDir) {
+ // directory recovery is not supported.
+ // log and fail.
+ LOG.info("rename directory {} to {} failed; unable to recover",
+ source, destination);
+ return false;
+ }
+ if (isNotEmpty(sourceEtag)) {
// Server has returned HTTP 404, which means rename source no longer
// exists. Check on destination status and if its etag matches
// that of the source, consider it to be a success.
- LOG.debug("rename {} to {} failed, checking etag of destination",
+ // the source tag was either passed in from a manifest commit or
+ // retrieved when rename recovery is enabled.
+ LOG.info("rename {} to {} failed, checking etag of destination",
source, destination);
try {
final AbfsRestOperation destStatusOp = getPathStatus(destination,
false, tracingContext);
final AbfsHttpOperation result = destStatusOp.getResult();
- return result.getStatusCode() == HttpURLConnection.HTTP_OK
+ final boolean recovered = result.getStatusCode() == HttpURLConnection.HTTP_OK
&& sourceEtag.equals(extractEtagHeader(result));
- } catch (AzureBlobFileSystemException ignored) {
+ LOG.info("File rename has taken place: recovery completed");
Review Comment:
this log statement is incorrect. It will depend on the value of the recovered flag.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a diff in pull request #5494: HADOOP-18425. ABFS rename recovery
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #5494:
URL: https://github.com/apache/hadoop/pull/5494#discussion_r1141965748
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -613,39 +644,58 @@ private void incrementAbfsRenamePath() {
* Exceptions raised in the probe of the destination are swallowed,
* so that they do not interfere with the original rename failures.
* @param source source path
+ * @param sourceEtag etag of source file. may be null or empty
* @param op Rename request REST operation response with non-null HTTP response
* @param destination rename destination path
- * @param sourceEtag etag of source file. may be null or empty
* @param tracingContext Tracks identifiers for request header
+ * @param isDir is the source a file or directory
* @return true if the file was successfully copied
*/
public boolean renameIdempotencyCheckOp(
final String source,
final String sourceEtag,
final AbfsRestOperation op,
final String destination,
- TracingContext tracingContext) {
+ TracingContext tracingContext,
+ final boolean isDir) {
Preconditions.checkArgument(op.hasResult(), "Operations has null HTTP response");
- if ((op.isARetriedRequest())
- && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)
- && isNotEmpty(sourceEtag)) {
-
+ if (!(op.isARetriedRequest())
+ && (op.getResult().getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND)) {
+ // not an error
+ return false;
+ }
+ LOG.debug("Source not found on retry of rename({}, {}) isDir {} etag {}",
+ source, destination, isDir, sourceEtag);
+ if (isDir) {
+ // directory recovery is not supported.
+ // log and fail.
+ LOG.info("rename directory {} to {} failed; unable to recover",
+ source, destination);
+ return false;
+ }
+ if (isNotEmpty(sourceEtag)) {
Review Comment:
ok. I do want this to work even when resilience is off, but passing in "" will do that, won't it?
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org