[GitHub] [hadoop] bgaborg commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename

2019-06-19 Thread GitBox
bgaborg 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_r295518287
 
 

 ##
 File path: 
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java
 ##
 @@ -138,9 +138,15 @@ private Constants() {
   public static final String ASSUMED_ROLE_CREDENTIALS_DEFAULT =
   SimpleAWSCredentialsProvider.NAME;
 
+
+  // the maximum number of tasks cached if all threads are already uploading
+  public static final String MAX_TOTAL_TASKS = "fs.s3a.max.total.tasks";
+
 
 Review comment:
   Nit: remove empty 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.
 
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] bgaborg commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename

2019-06-19 Thread GitBox
bgaborg 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_r295465643
 
 

 ##
 File path: 
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/LocalMetadataStore.java
 ##
 @@ -192,15 +202,19 @@ public synchronized DirListingMetadata listChildren(Path 
p) throws
   }
 
   @Override
-  public void move(Collection pathsToDelete,
-  Collection pathsToCreate,
-  ITtlTimeProvider ttlTimeProvider) throws IOException {
+  public void move(
+  @Nullable Collection pathsToDelete,
+  @Nullable Collection pathsToCreate,
+ITtlTimeProvider ttlTimeProvider,
 
 Review comment:
   Yes we did. I started a discussion about this if we want to pass it in the 
metastore init instead of every method which will use it. We ended up passing 
it to every method instead of the init method. We need to fix that. If that is 
not fixed in this pr I will fix it tomorrow under a new issue.


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] bgaborg commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename

2019-06-12 Thread GitBox
bgaborg 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_r292955268
 
 

 ##
 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:
   Maybe it would worth to create a separate method to the validation and all 
the other parts of this method that could be moved and tested separately from 
innerRename. This method is huge, 300+ lines; it would help the sustainability 
to split it up.


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] bgaborg commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename

2019-06-12 Thread GitBox
bgaborg 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_r293010385
 
 

 ##
 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 commented out test, or `@skip` with justification.


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] bgaborg commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename

2019-06-12 Thread GitBox
bgaborg 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_r292963979
 
 

 ##
 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:
   This is a lot of parameters for a constructor. I think it would worth to use 
builder pattern for readability and sustainability.


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] bgaborg commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename

2019-06-12 Thread GitBox
bgaborg 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_r293006629
 
 

 ##
 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:
   why do you remove old tests? just asking because of backward compatibility; 
maybe makes sense.


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] bgaborg commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename

2019-06-12 Thread GitBox
bgaborg 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_r293001205
 
 

 ##
 File path: 
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3Guard.java
 ##
 @@ -149,14 +148,63 @@ public static MetadataStore getMetadataStore(FileSystem 
fs)
   public static S3AFileStatus putAndReturn(MetadataStore ms,
   S3AFileStatus status,
   S3AInstrumentation instrumentation) throws IOException {
+return putAndReturn(ms, status, instrumentation, null);
+  }
+
+  /**
+   * Helper function which puts a given S3AFileStatus into the MetadataStore 
and
+   * returns the same S3AFileStatus. Instrumentation monitors the put 
operation.
+   * @param ms MetadataStore to {@code put()} into.
+   * @param status status to store
+   * @param instrumentation instrumentation of the s3a file system
+   * @param operationState possibly-null metastore state tracker.
+   * @return The same status as passed in
+   * @throws IOException if metadata store update failed
+   */
+  @RetryTranslated
+  public static S3AFileStatus putAndReturn(
+  final MetadataStore ms,
+  final S3AFileStatus status,
+  final S3AInstrumentation instrumentation,
+  @Nullable final BulkOperationState operationState) throws IOException {
 long startTimeNano = System.nanoTime();
-ms.put(new PathMetadata(status));
-instrumentation.addValueToQuantiles(S3GUARD_METADATASTORE_PUT_PATH_LATENCY,
-(System.nanoTime() - startTimeNano));
-instrumentation.incrementCounter(S3GUARD_METADATASTORE_PUT_PATH_REQUEST, 
1);
+try {
+  ms.put(new PathMetadata(status), operationState);
+} finally {
 
 Review comment:
   maybe add a log message on exception and rethrow


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] bgaborg commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename

2019-06-12 Thread GitBox
bgaborg 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_r293003924
 
 

 ##
 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:
   as I see this is an entirely different issue, which should be resolved in a 
separate commit. do you plan to do that instead of squashing all your changes 
to the same commit?


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] bgaborg commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename

2019-06-12 Thread GitBox
bgaborg 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_r292967396
 
 

 ##
 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:
   nit: if it's one parameter per line we should keep that way (add len to a 
new 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.
 
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] bgaborg commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename

2019-06-12 Thread GitBox
bgaborg 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_r292972748
 
 

 ##
 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:
   this would worth a builder pattern, and as this is new with this PR it will 
scale well in the future.


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 

[GitHub] [hadoop] bgaborg commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename

2019-06-12 Thread GitBox
bgaborg 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_r292991041
 
 

 ##
 File path: 
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/DynamoDBMetadataStore.java
 ##
 @@ -748,24 +942,45 @@ public void move(Collection pathsToDelete,
 // Following code is to maintain this invariant by putting all ancestor
 // directories of the paths to create.
 // ancestor paths that are not explicitly added to paths to create
-Collection newItems = new ArrayList<>();
+AncestorState ancestorState = extractOrCreate(operationState,
+BulkOperationState.OperationType.Rename);
+List newItems = new ArrayList<>();
 if (pathsToCreate != null) {
-  newItems.addAll(completeAncestry(pathMetaToDDBPathMeta(pathsToCreate)));
+  // create all parent entries.
+  // this is synchronized on the move state so that across both serialized
+  // and parallelized renames, duplicate ancestor entries are not created.
+  synchronized (ancestorState) {
+newItems.addAll(
+completeAncestry(
+pathMetaToDDBPathMeta(pathsToCreate),
+ancestorState));
+  }
 }
+// sort all the new items topmost first.
+newItems.sort(PathOrderComparators.TOPMOST_PM_FIRST);
+
+// now process the deletions.
 if (pathsToDelete != null) {
+  List tombstones = new ArrayList<>(pathsToDelete.size());
   for (Path meta : pathsToDelete) {
-newItems.add(new DDBPathMetadata(PathMetadata.tombstone(meta)));
+tombstones.add(new DDBPathMetadata(PathMetadata.tombstone(meta)));
   }
+  // sort all the tombstones lowest first.
+  tombstones.sort(PathOrderComparators.TOPMOST_PM_LAST);
+  newItems.addAll(tombstones);
 }
 
 processBatchWriteRequest(null, pathMetadataToItem(newItems));
   }
 
   /**
* Helper method to issue a batch write request to DynamoDB.
-   *
+   * 
+   *   Keys to delete are processed ahead of writing new items.
+   *   No attempt is made to sort the input: the caller must do that
+   * 
* As well as retrying on the operation invocation, incomplete
-   * batches are retried until all have been deleted.
+   * batches are retried until all have been processed..
 
 Review comment:
   nit: double dots


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] bgaborg commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename

2019-06-12 Thread GitBox
bgaborg 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_r293005305
 
 

 ##
 File path: 
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractRename.java
 ##
 @@ -64,12 +88,55 @@ public void testRenameDirIntoExistingDir() throws 
Throwable {
 Path destDir = path("dest");
 
 Path destFilePath = new Path(destDir, "dest-512.txt");
-byte[] destDateset = dataset(512, 'A', 'Z');
-writeDataset(fs, destFilePath, destDateset, destDateset.length, 1024,
+byte[] destDataset = dataset(512, 'A', 'Z');
+writeDataset(fs, destFilePath, destDataset, destDataset.length, 1024,
 false);
 assertIsFile(destFilePath);
 
 boolean rename = fs.rename(srcDir, destDir);
 assertFalse("s3a doesn't support rename to non-empty directory", rename);
   }
+
+  /**
+   * Test that after renaming, the nested file is moved along with all its
+   * ancestors. It is similar to {@link 
#testRenamePopulatesDirectoryAncestors}.
+   *
+   * This is an extension testRenamePopulatesFileAncestors
+   * of the superclass version which does better
+   * logging of the state of the store before the assertions.
+   */
+  @Test
+  public void testRenamePopulatesFileAncestors2() throws Exception {
+final S3AFileSystem fs = (S3AFileSystem)getFileSystem();
 
 Review comment:
   nit: space after cast


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] bgaborg commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename

2019-06-12 Thread GitBox
bgaborg 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_r292969539
 
 

 ##
 File path: 
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/commit/CommitOperations.java
 ##
 @@ -520,6 +533,120 @@ public void jobCompleted(boolean success) {
 statistics.jobCompleted(success);
   }
 
+  /**
+   * Begin the final commit.
+   * @param path path for all work.
+   * @return the commit context to pass in.
+   * @throws IOException failure.
+   */
+  public CommitContext initiateCommitOperation(Path path) throws IOException {
+return new CommitContext(writeOperations.initiateCommitOperation(path));
+  }
+
+  /**
+   * Commit context.
+   *
+   * It is used to manage the final commit sequence where files become
+   * visible. It contains a {@link BulkOperationState} field, which, if
+   * there is a metastore, will be requested from the store so that it
+   * can track multiple creation operations within the same overall operation.
+   * This will be null if there is no metastore, or the store chooses not
+   * to provide one.
+   *
+   * This can only be created through {@link #initiateCommitOperation(Path)}.
+   *
+   * Once the commit operation has completed, it must be closed.
+   * It must not be reused.
+   */
+  public final class CommitContext implements Closeable {
 
 Review comment:
   This could go to a separate file, so CommitOperations could be shorter and 
easier to overview.


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] bgaborg commented on a change in pull request #951: HADOOP-15183. S3Guard store becomes inconsistent after partial failure of rename

2019-06-12 Thread GitBox
bgaborg 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_r293011616
 
 

 ##
 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:
   Note: This test class is empty.


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