Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ public class AbfsCountersImpl implements AbfsCounters {
READ_THROTTLES,
WRITE_THROTTLES,
SERVER_UNAVAILABLE

};

private static final AbfsStatistic[] DURATION_TRACKER_LIST = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,14 @@ public enum AbfsStatistic {
AbfsHttpConstants.HTTP_METHOD_PATCH),
HTTP_POST_REQUEST(StoreStatisticNames.ACTION_HTTP_POST_REQUEST,
"Time taken to complete a POST request",
AbfsHttpConstants.HTTP_METHOD_POST);
AbfsHttpConstants.HTTP_METHOD_POST),

// Rename recovery
RENAME_RECOVERY("rename_recovery",
"Number of times Rename recoveries happened"),
METADATA_INCOMPLETE_FAILURES("metadata_incomplete_failures",
Comment thread
mehakmeet marked this conversation as resolved.
Outdated
"Number of times rename operation failed due to metadata being "
+ "incomplete");

private String statName;
private String statDescription;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import java.util.concurrent.TimeUnit;

import org.apache.hadoop.classification.VisibleForTesting;
import org.apache.hadoop.fs.azurebfs.services.AbfsClientResult;
import org.apache.hadoop.util.Preconditions;
import org.apache.hadoop.thirdparty.com.google.common.base.Strings;
import org.apache.hadoop.thirdparty.com.google.common.util.concurrent.Futures;
Expand Down Expand Up @@ -132,6 +133,8 @@
import org.apache.hadoop.util.concurrent.HadoopExecutors;
import org.apache.http.client.utils.URIBuilder;

import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.METADATA_INCOMPLETE_FAILURES;
import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.RENAME_RECOVERY;
import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.CHAR_EQUALS;
import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.CHAR_FORWARD_SLASH;
import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.CHAR_HYPHEN;
Expand Down Expand Up @@ -919,18 +922,19 @@ public boolean rename(final Path source,

do {
try (AbfsPerfInfo perfInfo = startTracking("rename", "renamePath")) {
final Pair<AbfsRestOperation, Boolean> pair =
final AbfsClientResult abfsClientResult =
client.renamePath(sourceRelativePath, destinationRelativePath,
continuation, tracingContext, sourceEtag);

AbfsRestOperation op = pair.getLeft();
AbfsRestOperation op = abfsClientResult.getOp();
perfInfo.registerResult(op.getResult());
continuation = op.getResult().getResponseHeader(HttpHeaderConfigurations.X_MS_CONTINUATION);
perfInfo.registerSuccess(true);
countAggregate++;
shouldContinue = continuation != null && !continuation.isEmpty();
// update the recovery flag.
recovered |= pair.getRight();
recovered |= abfsClientResult.isRenameRecovered();
populateRenameRecoveryStatistics(abfsClientResult);
if (!shouldContinue) {
perfInfo.registerAggregates(startAggregate, countAggregate);
}
Expand Down Expand Up @@ -1973,4 +1977,17 @@ public static String extractEtagHeader(AbfsHttpOperation result) {
}
return etag;
}

/**
* Increment Rename recovery based counters in IOStatistics.
* @param abfsClientResult Result of an ABFS operation.
*/
private void populateRenameRecoveryStatistics(AbfsClientResult abfsClientResult) {
if(abfsClientResult.isRenameRecovered()) {
Comment thread
mehakmeet marked this conversation as resolved.
Outdated
abfsCounters.incrementCounter(RENAME_RECOVERY, 1);
}
if(abfsClientResult.isIncompleteMetadataState()) {
abfsCounters.incrementCounter(METADATA_INCOMPLETE_FAILURES, 1);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import java.util.concurrent.TimeUnit;

import org.apache.hadoop.classification.VisibleForTesting;
import org.apache.hadoop.fs.store.LogExactlyOnce;
import org.apache.hadoop.util.Preconditions;
import org.apache.hadoop.thirdparty.com.google.common.base.Strings;
import org.apache.hadoop.thirdparty.com.google.common.util.concurrent.FutureCallback;
Expand All @@ -51,7 +52,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import org.apache.commons.lang3.tuple.Pair;
import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants;
import org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations;
import org.apache.hadoop.fs.azurebfs.constants.HttpQueryParams;
Expand All @@ -76,6 +76,7 @@
import static org.apache.hadoop.fs.azurebfs.constants.FileSystemUriSchemes.HTTPS_SCHEME;
import static org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations.*;
import static org.apache.hadoop.fs.azurebfs.constants.HttpQueryParams.*;
import static org.apache.hadoop.fs.azurebfs.contracts.services.AzureServiceErrorCode.RENAME_DESTINATION_PARENT_PATH_NOT_FOUND;

/**
* AbfsClient.
Expand All @@ -102,6 +103,16 @@ public class AbfsClient implements Closeable {

private final ListeningScheduledExecutorService executorService;

/**
* Is Abfs metadata been in an incomplete State resulting in a rename
* failure?
*/
private boolean isMetadataIncompleteState;

/** logging the rename failure if metadata is in an incomplete state*/
Comment thread
mehakmeet marked this conversation as resolved.
Outdated
private static final LogExactlyOnce ABFS_METADATA_INCOMPLETE_RENAME_FAILURE =
new LogExactlyOnce(LOG);

private AbfsClient(final URL baseUrl, final SharedKeyCredentials sharedKeyCredentials,
final AbfsConfiguration abfsConfiguration,
final AbfsClientContext abfsClientContext)
Expand Down Expand Up @@ -499,7 +510,7 @@ public AbfsRestOperation breakLease(final String path,
* @return pair of (the rename operation, flag indicating recovery took place)
* @throws AzureBlobFileSystemException failure, excluding any recovery from overload failures.
*/
public Pair<AbfsRestOperation, Boolean> renamePath(
public AbfsClientResult renamePath(
final String source,
final String destination,
final String continuation,
Expand Down Expand Up @@ -532,12 +543,35 @@ public Pair<AbfsRestOperation, Boolean> renamePath(
requestHeaders);
try {
op.execute(tracingContext);
return Pair.of(op, false);
// AbfsClientResult contains the AbfsOperation, If recovery happened or
// not, and the incompleteMetaDataState is true or false.
return new AbfsClientResult(op, isMetadataIncompleteState ? true : false, isMetadataIncompleteState);
Comment thread
mehakmeet marked this conversation as resolved.
Outdated
} catch (AzureBlobFileSystemException e) {
// If we have no HTTP response, throw the original exception.
if (!op.hasResult()) {
throw e;
}

Comment thread
steveloughran marked this conversation as resolved.
// ref: HADOOP-18242. Rename failure occurring due to a rare case of
// tracking metadata being in incomplete state.
if (op.getResult().getStorageErrorCode()
.equals(RENAME_DESTINATION_PARENT_PATH_NOT_FOUND.getErrorCode())
&& !isMetadataIncompleteState) {
//Logging
ABFS_METADATA_INCOMPLETE_RENAME_FAILURE
.info("Rename Failure attempting to resolve tracking metadata state and retrying.");

// Doing a HEAD call resolves the incomplete metadata state and
// then we can retry the rename operation.
getPathStatus(source, false, tracingContext);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've had one more thought here. the path status contains the etag, doesn't it? so if sourceEtag was null, now we can set it. That way if the rename failure is followed
immediately buy the rename-failure-but-it-really-happened event of HADOOP-18163, we are lined up for recovery

isMetadataIncompleteState = true;
renamePath(source, destination, continuation, tracingContext,
sourceEtag);
Comment thread
mehakmeet marked this conversation as resolved.
Outdated
}
// TODO: The case when Parent dir really don't exist, even after
// retrying once, we come at this line, should we return the result
// with recovery = false, metadata = true or throw an exception?

boolean etagCheckSucceeded = renameIdempotencyCheckOp(
source,
sourceEtag, op, destination, tracingContext);
Expand All @@ -546,7 +580,7 @@ public Pair<AbfsRestOperation, Boolean> renamePath(
// throw back the exception
throw e;
}
return Pair.of(op, true);
return new AbfsClientResult(op, true, isMetadataIncompleteState);
}
}

Expand Down Expand Up @@ -1243,4 +1277,9 @@ public ListenableFuture<?> submit(Runnable runnable) {
public <V> void addCallback(ListenableFuture<V> future, FutureCallback<V> callback) {
Futures.addCallback(future, callback, executorService);
}

@VisibleForTesting
public boolean isMetadataIncompleteState() {
return isMetadataIncompleteState;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/**
* 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.fs.azurebfs.services;

/**
* A class to store the Result of an AbfsClient Operation, signifying the
* AbfsRestOperation and the rename recovery.
Comment thread
mehakmeet marked this conversation as resolved.
Outdated
*/
public class AbfsClientResult {

/** Abfs Rest Operation. */
private final AbfsRestOperation op;
/** Flag indicating recovery took place. */
private final boolean renameRecovered;
/** Abfs storage tracking metadata is in an incomplete state.*/
private final boolean isIncompleteMetadataState;

public AbfsClientResult(
Comment thread
steveloughran marked this conversation as resolved.
Outdated
AbfsRestOperation op, boolean renameRecovered,
boolean isIncompleteMetadataState) {
this.op = op;
this.renameRecovered = renameRecovered;
this.isIncompleteMetadataState = isIncompleteMetadataState;
}

public AbfsRestOperation getOp() {
return op;
}

public boolean isRenameRecovered() {
return renameRecovered;
}

public boolean isIncompleteMetadataState() {
return isIncompleteMetadataState;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ public void testSignatureMask() throws Exception {
AbfsRestOperation abfsHttpRestOperation = fs.getAbfsClient()
.renamePath(src, "/testABC" + "/abc.txt", null,
getTestTracingContext(fs, false), null)
.getLeft();
.getOp();
AbfsHttpOperation result = abfsHttpRestOperation.getResult();
String url = result.getMaskedUrl();
String encodedUrl = result.getMaskedEncodedUrl();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertPathDoesNotExist;
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertPathExists;
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertRenameOutcome;
import static org.apache.hadoop.fs.contract.ContractTestUtils.dataset;
import static org.apache.hadoop.fs.contract.ContractTestUtils.writeDataset;

/**
* Test rename operation.
Expand Down Expand Up @@ -167,4 +169,31 @@ public void testPosixRenameDirectory() throws Exception {
new Path(testDir2 + "/test1/test2/test3"));
}

@Test
public void testRenameWithNoDestinationParentDir() throws Exception {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add similar test case for resilient rename api, here or in ITestAbfsManifestStoreOperations

describe("Verifying the expected behaviour of ABFS rename when "
+ "destination parent Dir doesn't exist.");

final AzureBlobFileSystem fs = getFileSystem();
Path sourcePath = path(getMethodName());
Path destPath = new Path("falseParent", "someChildFile");

byte[] data = dataset(1024, 'a', 'z');
writeDataset(fs, sourcePath, data, data.length, 1024, true);

// Check if we have seen an incomplete state.
boolean hasRenameRetriedOnce = fs.getAbfsClient().isMetadataIncompleteState();
assertFalse("No incomplete state should be seen before attempting to "
+ "rename",
hasRenameRetriedOnce);

// Verify that Renaming on a destination with no parent dir wasn't
Comment thread
mehakmeet marked this conversation as resolved.
Outdated
// successful.
assertFalse(fs.rename(sourcePath, destPath));

// Verify that metadata was in an incomplete state after the rename failure.
hasRenameRetriedOnce = fs.getAbfsClient().isMetadataIncompleteState();
Comment thread
mehakmeet marked this conversation as resolved.
Outdated
assertTrue("Rename should be retried once",
hasRenameRetriedOnce);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ private void testRenamePath(final boolean isWithCPK) throws Exception {
AbfsRestOperation abfsRestOperation = abfsClient
.renamePath(testFileName, newName, null,
getTestTracingContext(fs, false), null)
.getLeft();
.getOp();
assertCPKHeaders(abfsRestOperation, false);
assertNoCPKResponseHeadersPresent(abfsRestOperation);

Expand Down
Loading