[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard
store becomes inconsistent after partial failure of rename
URL: https://github.com/apache/hadoop/pull/951#discussion_r295835905
##
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
##
@@ -689,6 +707,7 @@ public String getBucketLocation() throws IOException {
* @return the region in which a bucket is located
* @throws IOException on any failure.
*/
+ @VisibleForTesting
Review comment:
The new storecontext API exports this as an on-demand operation.:
`fs.createStoreContext().getBucketLocation()`
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard
store becomes inconsistent after partial failure of rename
URL: https://github.com/apache/hadoop/pull/951#discussion_r294314480
##
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DynamoDBMetadataStore.java
##
@@ -699,39 +769,168 @@ DirListingMetadata
getDirListingMetadataFromDirMetaAndList(Path path,
}
/**
- * build the list of all parent entries.
+ * Build the list of all parent entries.
+ *
+ * Thread safety: none. Callers must synchronize access.
+ *
+ * Callers are required to synchronize on ancestorState.
* @param pathsToCreate paths to create
+ * @param ancestorState ongoing ancestor state.
* @return the full ancestry paths
*/
- Collection completeAncestry(
- Collection pathsToCreate) {
-// Key on path to allow fast lookup
-Map ancestry = new HashMap<>();
-
-for (DDBPathMetadata meta : pathsToCreate) {
+ private Collection completeAncestry(
+ final Collection pathsToCreate,
+ final AncestorState ancestorState) throws PathIOException {
+List ancestorsToAdd = new ArrayList<>(0);
+LOG.debug("Completing ancestry for {} paths", pathsToCreate.size());
+// we sort the inputs to guarantee that the topmost entries come first.
+// that way if the put request contains both parents and children
+// then the existing parents will not be re-created -they will just
+// be added to the ancestor list first.
+List sortedPaths = new ArrayList<>(pathsToCreate);
+sortedPaths.sort(PathOrderComparators.TOPMOST_PM_FIRST);
+for (DDBPathMetadata meta : sortedPaths) {
Preconditions.checkArgument(meta != null);
Path path = meta.getFileStatus().getPath();
+ LOG.debug("Adding entry {}", path);
if (path.isRoot()) {
break;
}
- ancestry.put(path, new DDBPathMetadata(meta));
+ // add the new entry
+ DDBPathMetadata entry = new DDBPathMetadata(meta);
+ DDBPathMetadata oldEntry = ancestorState.put(path, entry);
+ if (oldEntry != null) {
+if (!oldEntry.getFileStatus().isDirectory()
+|| !entry.getFileStatus().isDirectory()) {
+ // check for and warn if the existing bulk operation overwrote it.
+ // this should never occur outside tests explicitly crating it
+ LOG.warn("Overwriting a S3Guard entry created in the operation: {}",
+ oldEntry);
+ LOG.warn("With new entry: {}", entry);
+ // restore the old state
+ ancestorState.put(path, oldEntry);
+ // then raise an exception
+ throw new PathIOException(path.toString(), E_INCONSISTENT_UPDATE);
+} else {
+ // directory is already present, so skip adding it and any parents.
+ continue;
+}
+ }
+ ancestorsToAdd.add(entry);
Path parent = path.getParent();
- while (!parent.isRoot() && !ancestry.containsKey(parent)) {
+ while (!parent.isRoot()) {
+if (ancestorState.findEntry(parent, true)) {
+ break;
+}
LOG.debug("auto-create ancestor path {} for child path {}",
parent, path);
final S3AFileStatus status = makeDirStatus(parent, username);
-ancestry.put(parent, new DDBPathMetadata(status, Tristate.FALSE,
-false));
+DDBPathMetadata md = new DDBPathMetadata(status, Tristate.FALSE,
+false);
+ancestorState.put(parent, md);
+ancestorsToAdd.add(md);
parent = parent.getParent();
}
}
-return ancestry.values();
+return ancestorsToAdd;
+ }
+
+ /**
+ * {@inheritDoc}
+ *
+ * if {@code operationState} is not null, when this method returns the
+ * operation state will be updated with all new entries created.
+ * This ensures that subsequent operations with the same store will not
+ * trigger new updates.
+ * The scan on
Review comment:
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename URL: https://github.com/apache/hadoop/pull/951#discussion_r294314157 ## File path: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3GuardTool.java ## @@ -1134,16 +1147,19 @@ public int run(String[] args, PrintStream out) } String s3Path = paths.get(0); CommandFormat commands = getCommandFormat(); + URI fsURI = toUri(s3Path); // check if UNGUARDED_FLAG is passed and use NullMetadataStore in // config to avoid side effects like creating the table if not exists + Configuration conf0 = getConf(); Review comment: done. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename URL: https://github.com/apache/hadoop/pull/951#discussion_r293514874 ## File path: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/impl/ITestPartialRenamesDeletes.java ## @@ -0,0 +1,871 @@ +/* + * 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.s3a.impl; + +import java.io.IOException; +import java.nio.file.AccessDeniedException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import com.amazonaws.services.s3.model.MultiObjectDeleteException; +import com.google.common.base.Charsets; +import com.google.common.util.concurrent.ListeningExecutorService; +import org.assertj.core.api.Assertions; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileStatus; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.fs.contract.ContractTestUtils; +import org.apache.hadoop.fs.s3a.AbstractS3ATestBase; +import org.apache.hadoop.fs.s3a.S3AFileSystem; +import org.apache.hadoop.fs.s3a.S3AUtils; +import org.apache.hadoop.fs.s3a.s3guard.MetadataStore; +import org.apache.hadoop.fs.s3a.s3guard.PathMetadataDynamoDBTranslation; +import org.apache.hadoop.util.BlockingThreadPoolExecutorService; +import org.apache.hadoop.util.DurationInfo; + +import static org.apache.hadoop.fs.contract.ContractTestUtils.*; +import static org.apache.hadoop.fs.s3a.Constants.*; +import static org.apache.hadoop.fs.s3a.S3ATestUtils.MetricDiff; +import static org.apache.hadoop.fs.s3a.S3ATestUtils.*; +import static org.apache.hadoop.fs.s3a.S3AUtils.applyLocatedFiles; +import static org.apache.hadoop.fs.s3a.Statistic.FILES_DELETE_REJECTED; +import static org.apache.hadoop.fs.s3a.Statistic.OBJECT_DELETE_REQUESTS; +import static org.apache.hadoop.fs.s3a.auth.RoleModel.Effects; +import static org.apache.hadoop.fs.s3a.auth.RoleModel.Statement; +import static org.apache.hadoop.fs.s3a.auth.RoleModel.directory; +import static org.apache.hadoop.fs.s3a.auth.RoleModel.statement; +import static org.apache.hadoop.fs.s3a.auth.RolePolicies.*; +import static org.apache.hadoop.fs.s3a.auth.RoleTestUtils.bindRolePolicyStatements; +import static org.apache.hadoop.fs.s3a.auth.RoleTestUtils.forbidden; +import static org.apache.hadoop.fs.s3a.auth.RoleTestUtils.newAssumedRoleConfig; +import static org.apache.hadoop.fs.s3a.impl.CallableSupplier.submit; +import static org.apache.hadoop.fs.s3a.impl.CallableSupplier.waitForCompletion; +import static org.apache.hadoop.fs.s3a.impl.MultiObjectDeleteSupport.extractUndeletedPaths; +import static org.apache.hadoop.fs.s3a.impl.MultiObjectDeleteSupport.removeUndeletedPaths; +import static org.apache.hadoop.fs.s3a.test.ExtraAssertions.assertFileCount; +import static org.apache.hadoop.fs.s3a.test.ExtraAssertions.extractCause; +import static org.apache.hadoop.test.LambdaTestUtils.eval; + +/** + * Test partial failures of delete and rename operations, especially + * that the S3Guard tables are consistent with the state of + * the filesystem. + * + * All these test have a unique path for each run, with a roleFS having + * full RW access to part of it, and R/O access to a restricted subdirectory + * + * + * + * The tests are parameterized to single/multi delete, which control which + * of the two delete mechanisms are used. + * + * + * In multi delete, in a scale test run, a significantly larger set of files + * is created and then deleted. + * + * + * This isn't done in the single delete as it is much slower and it is not + * the situation we are trying to create. + * + * + * + * This test manages to create lots of load on the s3guard prune command + *
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard
store becomes inconsistent after partial failure of rename
URL: https://github.com/apache/hadoop/pull/951#discussion_r293514525
##
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DynamoDBMetadataStore.java
##
@@ -699,39 +769,168 @@ DirListingMetadata
getDirListingMetadataFromDirMetaAndList(Path path,
}
/**
- * build the list of all parent entries.
+ * Build the list of all parent entries.
+ *
+ * Thread safety: none. Callers must synchronize access.
+ *
+ * Callers are required to synchronize on ancestorState.
* @param pathsToCreate paths to create
+ * @param ancestorState ongoing ancestor state.
* @return the full ancestry paths
*/
- Collection completeAncestry(
- Collection pathsToCreate) {
-// Key on path to allow fast lookup
-Map ancestry = new HashMap<>();
-
-for (DDBPathMetadata meta : pathsToCreate) {
+ private Collection completeAncestry(
+ final Collection pathsToCreate,
+ final AncestorState ancestorState) throws PathIOException {
+List ancestorsToAdd = new ArrayList<>(0);
+LOG.debug("Completing ancestry for {} paths", pathsToCreate.size());
+// we sort the inputs to guarantee that the topmost entries come first.
+// that way if the put request contains both parents and children
+// then the existing parents will not be re-created -they will just
+// be added to the ancestor list first.
+List sortedPaths = new ArrayList<>(pathsToCreate);
+sortedPaths.sort(PathOrderComparators.TOPMOST_PM_FIRST);
+for (DDBPathMetadata meta : sortedPaths) {
Preconditions.checkArgument(meta != null);
Path path = meta.getFileStatus().getPath();
+ LOG.debug("Adding entry {}", path);
if (path.isRoot()) {
break;
}
- ancestry.put(path, new DDBPathMetadata(meta));
+ // add the new entry
+ DDBPathMetadata entry = new DDBPathMetadata(meta);
+ DDBPathMetadata oldEntry = ancestorState.put(path, entry);
+ if (oldEntry != null) {
+if (!oldEntry.getFileStatus().isDirectory()
+|| !entry.getFileStatus().isDirectory()) {
+ // check for and warn if the existing bulk operation overwrote it.
+ // this should never occur outside tests explicitly crating it
+ LOG.warn("Overwriting a S3Guard entry created in the operation: {}",
+ oldEntry);
+ LOG.warn("With new entry: {}", entry);
+ // restore the old state
+ ancestorState.put(path, oldEntry);
+ // then raise an exception
+ throw new PathIOException(path.toString(), E_INCONSISTENT_UPDATE);
+} else {
+ // directory is already present, so skip adding it and any parents.
+ continue;
+}
+ }
+ ancestorsToAdd.add(entry);
Path parent = path.getParent();
- while (!parent.isRoot() && !ancestry.containsKey(parent)) {
+ while (!parent.isRoot()) {
+if (ancestorState.findEntry(parent, true)) {
+ break;
+}
LOG.debug("auto-create ancestor path {} for child path {}",
parent, path);
final S3AFileStatus status = makeDirStatus(parent, username);
-ancestry.put(parent, new DDBPathMetadata(status, Tristate.FALSE,
-false));
+DDBPathMetadata md = new DDBPathMetadata(status, Tristate.FALSE,
+false);
+ancestorState.put(parent, md);
+ancestorsToAdd.add(md);
parent = parent.getParent();
}
}
-return ancestry.values();
+return ancestorsToAdd;
+ }
+
+ /**
+ * {@inheritDoc}
+ *
+ * if {@code operationState} is not null, when this method returns the
+ * operation state will be updated with all new entries created.
+ * This ensures that subsequent operations with the same store will not
+ * trigger new updates.
+ * The scan on
+ * @param qualifiedPath path to update
+ * @param operationState (nullable) operational state for a bulk update
+ * @throws IOException on failure.
+ */
+ @SuppressWarnings("SynchronizationOnLocalVariableOrMethodParameter")
+ @Override
+ @Retries.RetryTranslated
+ public void addAncestors(
+ final Path qualifiedPath,
+ @Nullable final BulkOperationState operationState) throws IOException {
+
+Collection newDirs = new ArrayList<>();
+final AncestorState ancestorState = extractOrCreate(operationState,
+BulkOperationState.OperationType.Rename);
+Path parent = qualifiedPath.getParent();
+
+// Iterate up the parents.
+// note that only ancestorState get/set operations are synchronized;
+// the DDB read between them is not. As a result, more than one
+// thread may probe the state, find the entry missing, do the database
+// query and add the entry.
+// This is done to avoid making the remote dynamo query part of the
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard
store becomes inconsistent after partial failure of rename
URL: https://github.com/apache/hadoop/pull/951#discussion_r293483871
##
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/ProgressiveRenameTracker.java
##
@@ -0,0 +1,245 @@
+/*
+ * 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.s3a.s3guard;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.s3a.S3ObjectAttributes;
+import org.apache.hadoop.fs.s3a.impl.StoreContext;
+import org.apache.hadoop.util.DurationInfo;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static org.apache.hadoop.fs.s3a.s3guard.S3Guard.addMoveAncestors;
+import static org.apache.hadoop.fs.s3a.s3guard.S3Guard.addMoveDir;
+
+/**
+ * This rename tracker progressively updates the metadata store
+ * as it proceeds, during the parallelized copy operation.
+ *
+ * Algorithm
+ *
+ *
+ * As {@code RenameTracker.fileCopied()} callbacks
+ * are raised, the metastore is updated with the new file entry.
+ *
+ *
+ * Including parent entries, as appropriate.
+ *
+ *
+ * All directories which have been created are tracked locally,
+ * to avoid needing to read the store; this is a thread-safe structure.
+ *
+ *
+ *The actual update is performed out of any synchronized block.
+ *
+ *
+ * When deletes are executed, the store is also updated.
+ *
+ *
+ * And at the completion of a successful rename, the source directory
+ * is also removed.
+ *
+ *
+ *
+ *
+ *
+ */
+public class ProgressiveRenameTracker extends RenameTracker {
+
+ /**
+ * The collection of paths to delete; this is added as individual files
+ * are renamed.
+ *
+ * The metastore is only updated with these entries after the DELETE
+ * call containing these paths succeeds.
+ *
+ * If the DELETE fails; the filesystem will use
+ * {@code MultiObjectDeleteSupport} to remove all successfully deleted
+ * entries from the metastore.
+ */
+ private final Collection pathsToDelete = new HashSet<>();
+
+ /**
+ * The list of new entries to add.
+ */
+ private final List destMetas = new ArrayList<>();
+
+ public ProgressiveRenameTracker(
+ final StoreContext storeContext,
+ final MetadataStore metadataStore,
+ final Path sourceRoot,
+ final Path dest,
+ final BulkOperationState operationState) {
+super("ProgressiveRenameTracker",
+storeContext, metadataStore, sourceRoot, dest, operationState);
+ }
+
+ /**
+ * When a file is copied, any ancestors
+ * are calculated and then the store is updated with
+ * the destination entries.
+ *
+ * The source entries are added to the {@link #pathsToDelete} list.
+ * @param sourcePath path of source
+ * @param sourceAttributes status of source.
+ * @param destAttributes destination attributes
+ * @param destPath destination path.
+ * @param blockSize block size.
+ * @param addAncestors should ancestors be added?
+ * @throws IOException failure
+ */
+ @Override
+ public void fileCopied(
+ final Path sourcePath,
+ final S3ObjectAttributes sourceAttributes,
+ final S3ObjectAttributes destAttributes,
+ final Path destPath,
+ final long blockSize,
+ final boolean addAncestors) throws IOException {
+
+// build the list of entries to add in a synchronized block.
+final List entriesToAdd = new ArrayList<>(1);
+LOG.debug("Updating store with copied file {}", sourcePath);
+MetadataStore store = getMetadataStore();
+synchronized (this) {
+ checkArgument(!pathsToDelete.contains(sourcePath),
+ "File being renamed is already processed %s", destPath);
+ // create the file metadata and update the local structures.
+ S3Guard.addMoveFile(
+ store,
+ pathsToDelete,
+ entriesToAdd,
+ sourcePath,
+ destPath,
+
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard
store becomes inconsistent after partial failure of rename
URL: https://github.com/apache/hadoop/pull/951#discussion_r293480453
##
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathOrderComparators.java
##
@@ -0,0 +1,133 @@
+/*
+ * 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.s3a.s3guard;
+
+import java.io.Serializable;
+import java.util.Comparator;
+
+import org.apache.hadoop.fs.Path;
+
+/**
+ * Comparator of path ordering for sorting collections.
+ *
+ * The definition of "topmost" is:
+ *
+ * The depth of a path is the primary comparator.
+ * Root is topmost, "0"
+ * If two paths are of equal depth, {@link Path#compareTo(Path)}
+ * is used. This delegates to URI compareTo.
+ * repeated sorts do not change the order
+ *
+ */
+final class PathOrderComparators {
+
+ private PathOrderComparators() {
+ }
+
+ /**
+ * The shallowest paths come first.
+ * This is to be used when adding entries.
+ */
+ static final Comparator TOPMOST_PATH_FIRST
+ = new TopmostFirst();
+
+ /**
+ * The leaves come first.
+ * This is to be used when deleting entries.
+ */
+ static final Comparator TOPMOST_PATH_LAST
+ = new TopmostLast();
+
+ /**
+ * The shallowest paths come first.
+ * This is to be used when adding entries.
+ */
+ static final Comparator TOPMOST_PM_FIRST
+ = new PathMetadataComparator(TOPMOST_PATH_FIRST);
+
+ /**
+ * The leaves come first.
+ * This is to be used when deleting entries.
+ */
+ static final Comparator TOPMOST_PM_LAST
+ = new PathMetadataComparator(TOPMOST_PATH_LAST);
+
+ private static class TopmostFirst implements Comparator, Serializable {
+
+@Override
+public int compare(Path pathL, Path pathR) {
+ // exist fast on equal values.
+ if (pathL.equals(pathR)) {
+return 0;
+ }
+ int depthL = pathL.depth();
+ int depthR = pathR.depth();
+ if (depthL < depthR) {
+// left is higher up than the right.
+return -1;
+ }
+ if (depthR < depthL) {
+// right is higher up than the left
+return 1;
+ }
+ // and if they are of equal depth, use the "classic" comparator
+ // of paths.
+ return pathL.compareTo(pathR);
+}
+ }
+
+ /**
+ * Compare the topmost last.
+ * For some reason the .reverse() option wasn't giving the
+ * correct outcome.
+ */
+ private static final class TopmostLast extends TopmostFirst {
+
+@Override
+public int compare(final Path pathL, final Path pathR) {
+ int compare = super.compare(pathL, pathR);
+ if (compare < 0) {
+return 1;
+ }
+ if (compare > 0) {
+return -1;
+ }
Review comment:
you'd think so, but when I tried my sort tests were failing -and even
stepping through the code with the debugger couldn't work out why. Doing in
like this fixed the tests
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename URL: https://github.com/apache/hadoop/pull/951#discussion_r293480057 ## File path: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/MetadataStore.java ## @@ -169,15 +186,33 @@ void move(Collection pathsToDelete, @RetryTranslated void put(PathMetadata meta) throws IOException; + /** + * Saves metadata for exactly one path, potentially + * using any bulk operation state to eliminate duplicate work. + * Review comment: None of the metastores are doing anything with delayed operations, just tracking it. Clarified in the javadocs 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard
store becomes inconsistent after partial failure of rename
URL: https://github.com/apache/hadoop/pull/951#discussion_r293480163
##
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/PathOrderComparators.java
##
@@ -0,0 +1,133 @@
+/*
+ * 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.s3a.s3guard;
+
+import java.io.Serializable;
+import java.util.Comparator;
+
+import org.apache.hadoop.fs.Path;
+
+/**
+ * Comparator of path ordering for sorting collections.
+ *
+ * The definition of "topmost" is:
+ *
+ * The depth of a path is the primary comparator.
+ * Root is topmost, "0"
+ * If two paths are of equal depth, {@link Path#compareTo(Path)}
+ * is used. This delegates to URI compareTo.
+ * repeated sorts do not change the order
+ *
+ */
+final class PathOrderComparators {
+
+ private PathOrderComparators() {
+ }
+
+ /**
+ * The shallowest paths come first.
+ * This is to be used when adding entries.
+ */
+ static final Comparator TOPMOST_PATH_FIRST
+ = new TopmostFirst();
+
+ /**
+ * The leaves come first.
+ * This is to be used when deleting entries.
+ */
+ static final Comparator TOPMOST_PATH_LAST
+ = new TopmostLast();
+
+ /**
+ * The shallowest paths come first.
+ * This is to be used when adding entries.
+ */
+ static final Comparator TOPMOST_PM_FIRST
+ = new PathMetadataComparator(TOPMOST_PATH_FIRST);
+
+ /**
+ * The leaves come first.
+ * This is to be used when deleting entries.
+ */
+ static final Comparator TOPMOST_PM_LAST
+ = new PathMetadataComparator(TOPMOST_PATH_LAST);
+
+ private static class TopmostFirst implements Comparator, Serializable {
+
+@Override
+public int compare(Path pathL, Path pathR) {
+ // exist fast on equal values.
Review comment:
done
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename URL: https://github.com/apache/hadoop/pull/951#discussion_r293478114 ## File path: hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/s3guard.md ## @@ -1474,6 +1474,18 @@ Caused by: java.lang.NullPointerException ... 1 more ``` +### Error `Attempt to change a resource which is still in use: Table is being deleted` Review comment: I do have branches of the unsquashed PRs, so I should be able to do that. By popular requeset I will give it a go, but leave them in here. If people get the other PR in first, I'll deal with that 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard
store becomes inconsistent after partial failure of rename
URL: https://github.com/apache/hadoop/pull/951#discussion_r293092407
##
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ObjectAttributes.java
##
@@ -18,38 +18,73 @@
package org.apache.hadoop.fs.s3a;
+import com.amazonaws.services.s3.transfer.model.CopyResult;
+
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.fs.Path;
/**
- * This class is only a holder for bucket, key, SSE Algorithm and SSE key
- * attributes. It is used in {@link S3AInputStream} and the select equivalent.
+ * This class holds attributed of an object independent of the
+ * file status type.
+ * It is used in {@link S3AInputStream} and the select equivalent.
* as a way to reduce parameters being passed
- * to the constructor of such class.
+ * to the constructor of such class,
+ * and elsewhere to be a source-neutral representation of a file status.
*/
@InterfaceAudience.Private
@InterfaceStability.Evolving
public class S3ObjectAttributes {
private final String bucket;
+ private final Path path;
private final String key;
private final S3AEncryptionMethods serverSideEncryptionAlgorithm;
private final String serverSideEncryptionKey;
private final String eTag;
private final String versionId;
+ private final long len;
public S3ObjectAttributes(
String bucket,
+ Path path,
String key,
S3AEncryptionMethods serverSideEncryptionAlgorithm,
String serverSideEncryptionKey,
String eTag,
- String versionId) {
+ String versionId, final long len) {
Review comment:
done. IDE refactoring at work
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard
store becomes inconsistent after partial failure of rename
URL: https://github.com/apache/hadoop/pull/951#discussion_r293091713
##
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
##
@@ -1225,130 +1302,292 @@ private boolean innerRename(Path source, Path dest)
}
}
-// If we have a MetadataStore, track deletions/creations.
-Collection srcPaths = null;
-List dstMetas = null;
-if (hasMetadataStore()) {
- srcPaths = new HashSet<>(); // srcPaths need fast look up before put
- dstMetas = new ArrayList<>();
-}
-// TODO S3Guard HADOOP-13761: retries when source paths are not visible yet
+// Validation completed: time to begin the operation.
Review comment:
done. Not doing any tests on the validation alone, as the contract tests are
expected to generate the failure conditions, but it does help isolate the two
stages
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard
store becomes inconsistent after partial failure of rename
URL: https://github.com/apache/hadoop/pull/951#discussion_r293082608
##
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
##
@@ -1225,130 +1302,292 @@ private boolean innerRename(Path source, Path dest)
}
}
-// If we have a MetadataStore, track deletions/creations.
-Collection srcPaths = null;
-List dstMetas = null;
-if (hasMetadataStore()) {
- srcPaths = new HashSet<>(); // srcPaths need fast look up before put
- dstMetas = new ArrayList<>();
-}
-// TODO S3Guard HADOOP-13761: retries when source paths are not visible yet
+// Validation completed: time to begin the operation.
Review comment:
That I will do. It might also line me up better for when I open up the
rename/3 operation which will always throw an exception on any invalid state
(rather than return "false" with no explanation)
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard
store becomes inconsistent after partial failure of rename
URL: https://github.com/apache/hadoop/pull/951#discussion_r293077850
##
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/StoreContext.java
##
@@ -0,0 +1,328 @@
+/*
+ * 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.s3a.impl;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.util.function.Function;
+
+import com.google.common.util.concurrent.ListeningExecutorService;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.s3a.Invoker;
+import org.apache.hadoop.fs.s3a.S3AInputPolicy;
+import org.apache.hadoop.fs.s3a.S3AInstrumentation;
+import org.apache.hadoop.fs.s3a.S3AStorageStatistics;
+import org.apache.hadoop.fs.s3a.Statistic;
+import org.apache.hadoop.fs.s3a.s3guard.MetadataStore;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.hadoop.util.SemaphoredDelegatingExecutor;
+
+/**
+ * This class provides the core context of the S3A filesystem to subsidiary
+ * components, without exposing the entire parent class.
+ * This is eliminate explicit recursive coupling.
+ *
+ * Where methods on the FS are to be invoked, they are all passed in
+ * via functional interfaces, so test setups can pass in mock callbacks
+ * instead.
+ *
+ * Warning: this really is private and unstable. Do not use
+ * outside the org.apache.hadoop.fs.s3a package.
+ */
+@InterfaceAudience.Private
+@InterfaceStability.Unstable
+public class StoreContext {
+
+ /** Filesystem URI. */
+ private final URI fsURI;
+
+ /** Bucket name. */
+ private final String bucket;
+
+ /** FS configuration after all per-bucket overrides applied. */
+ private final Configuration configuration;
+
+ /** Username. */
+ private final String username;
+
+ /** Principal who created the FS. */
+ private final UserGroupInformation owner;
+
+ /**
+ * Bounded thread pool for async operations.
+ */
+ private final ListeningExecutorService executor;
+
+ /**
+ * Capacity of new executors created.
+ */
+ private final int executorCapacity;
+
+ /** Invoker of operations. */
+ private final Invoker invoker;
+
+ /** Instrumentation and statistics. */
+ private final S3AInstrumentation instrumentation;
+ private final S3AStorageStatistics storageStatistics;
+
+ /** Seek policy. */
+ private final S3AInputPolicy inputPolicy;
+
+ /** How to react to changes in etags and versions. */
+ private final ChangeDetectionPolicy changeDetectionPolicy;
+
+ /** Evaluated options. */
+ private final boolean multiObjectDeleteEnabled;
+
+ /** List algorithm. */
+ private final boolean useListV1;
+
+ /** Is the store versioned? */
+ private final boolean versioned;
+
+ /**
+ * To allow this context to be passed down to the metastore, this field
+ * wll be null until initialized.
+ */
+ private final MetadataStore metadataStore;
+
+ /** Function to take a key and return a path. */
+ private final Function keyToPathQualifier;
+
+ /** Factory for temporary files. */
+ private final FunctionsRaisingIOE.BiFunctionRaisingIOE
+ tempFileFactory;
+
+ private final FunctionsRaisingIOE.CallableRaisingIOE
+ getBucketLocation;
+
+ /**
+ * Instantiate.
+ * No attempt to use a builder here as outside tests
+ * this should only be created in the S3AFileSystem.
+ */
+ public StoreContext(
Review comment:
No, for the reasons as discussed.
1. if we add more args, we want 100% of uses (production, test cases) to add
every param. Doing that in the refactor operations of the IDE is
straightforward.
2. I'm thinking that as more FS-level operations are added (path to key,
temp file...) I'd just add a new interface "FSLevelOperations" which we'd
implement in S3A and for any test suite. This would avoid the need for a new
field & constructor arg on every operation, though I'd still require code to
call an explicit
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard
store becomes inconsistent after partial failure of rename
URL: https://github.com/apache/hadoop/pull/951#discussion_r293071377
##
File path:
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestProgressiveRenameTracker.java
##
@@ -0,0 +1,25 @@
+/*
+ * 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.s3a.s3guard;
+
+/**
+ * Unit tests for {@link ProgressiveRenameTracker}.
+ */
+public class TestProgressiveRenameTracker {
Review comment:
oops :)
I'd started it as a placeholder for some unit tests, but clearly didn't
write them. Deleting the case
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard
store becomes inconsistent after partial failure of rename
URL: https://github.com/apache/hadoop/pull/951#discussion_r293071377
##
File path:
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/TestProgressiveRenameTracker.java
##
@@ -0,0 +1,25 @@
+/*
+ * 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.s3a.s3guard;
+
+/**
+ * Unit tests for {@link ProgressiveRenameTracker}.
+ */
+public class TestProgressiveRenameTracker {
Review comment:
oops :)
I'd started it as a placeholder for some unit tests, but clearly didn't
write them. Deleting the class
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard
store becomes inconsistent after partial failure of rename
URL: https://github.com/apache/hadoop/pull/951#discussion_r293070990
##
File path:
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/ITestDynamoDBMetadataStore.java
##
@@ -725,50 +975,55 @@ public void testDeleteTable() throws Exception {
intercept(IOException.class, "",
"Should have failed after the table is destroyed!",
() -> ddbms.listChildren(testPath));
-} finally {
ddbms.destroy();
- ddbms.close();
+ intercept(FileNotFoundException.class, "",
+ "Destroyed table should raise FileNotFoundException when pruned",
+ () -> ddbms.prune(0));
+} finally {
+ destroy(ddbms);
}
}
+/*
Review comment:
remove. I'd originally moved the tagging code into the static, then pulled
it into the aggregate testTableVersioning which combines 4 tests which were
creating and destroying individual tables
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard
store becomes inconsistent after partial failure of rename
URL: https://github.com/apache/hadoop/pull/951#discussion_r293070136
##
File path:
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/auth/ITestAssumeRole.java
##
@@ -524,78 +521,6 @@ public Path methodPath() throws IOException {
return path(getMethodName());
}
- @Test
- public void testRestrictedRename() throws Throwable {
-describe("rename with parent paths not writeable");
-executeRestrictedRename(createAssumedRoleConfig());
- }
-
- @Test
- public void testRestrictedSingleDeleteRename() throws Throwable {
Review comment:
moved to ITestPartialRenamesDeletes and extended, parameterized etc.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard
store becomes inconsistent after partial failure of rename
URL: https://github.com/apache/hadoop/pull/951#discussion_r293067787
##
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
##
@@ -2359,6 +2708,37 @@ public UserGroupInformation getOwner() {
return owner;
}
+ /**
+ * Build an immutable store context.
+ * If called while the FS is being initialized,
+ * some of the context will be incomplete.
+ * new store context instances should be created as appropriate.
+ * @return the store context of this FS.
+ */
+ @InterfaceAudience.Private
+ public StoreContext createStoreContext() {
Review comment:
I must disagree. The builder pattern is best for when you want to have
partial config or support change where you dont want to add many, many
constructors, and substitutes for Java's lack or named params in constructors
(compare with: groovy, scala, python)
Here: all parameters must be supplied and this is exclusively for use in the
s3a connector. Nobody should be creating these elsewhere, and if they do, not
my problem if it breaks
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename
steveloughran commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename URL: https://github.com/apache/hadoop/pull/951#discussion_r293059118 ## File path: hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/s3guard.md ## @@ -1474,6 +1474,18 @@ Caused by: java.lang.NullPointerException ... 1 more ``` +### Error `Attempt to change a resource which is still in use: Table is being deleted` Review comment: I've been working on this patch for too long and wrapping up stuff to try and get all tests to worse; a lot of work has gone into the ITestDynamoDB there. I don't want to split them up at this late in the patch. Sorry This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
