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 + * + * <ol> + * <li> + * The tests are parameterized to single/multi delete, which control which + * of the two delete mechanisms are used. + * </li> + * <li> + * In multi delete, in a scale test run, a significantly larger set of files + * is created and then deleted. + * </li> + * <li> + * This isn't done in the single delete as it is much slower and it is not + * the situation we are trying to create. + * </li> + * </ol> + * + * This test manages to create lots of load on the s3guard prune command + * when that is tested in a separate test suite; + * too many tombstone files for the test to complete. + * An attempt is made in {@link #deleteTestDirInTeardown()} to prune these test + * files. + */ +@SuppressWarnings("ThrowableNotThrown") +@RunWith(Parameterized.class) +public class ITestPartialRenamesDeletes extends AbstractS3ATestBase { + + private static final Logger LOG = + LoggerFactory.getLogger(ITestPartialRenamesDeletes.class); + + private static final Path ROOT = new Path("/"); + + private static final Statement STATEMENT_ALL_BUCKET_READ_ACCESS + = statement(true, S3_ALL_BUCKETS, S3_BUCKET_READ_OPERATIONS); + + /** Many threads for scale performance: {@value}. */ + public static final int EXECUTOR_THREAD_COUNT = 64; + + /** + * For submitting work. + */ + private static final ListeningExecutorService EXECUTOR = + BlockingThreadPoolExecutorService.newInstance( + EXECUTOR_THREAD_COUNT, + EXECUTOR_THREAD_COUNT * 2, + 30, TimeUnit.SECONDS, + "test-operations"); + + + /** + * The number of files in a non-scaled test. + * <p> + * Value: {@value}. + */ + public static final int FILE_COUNT_NON_SCALED = 2; + + /** + * The number of files for a scaled test. This is still + * less than half the amount which can be fitted into a delete + * request, so that even with this many R/W and R/O files, + * both can fit in the same request. + * Then, when a partial delete occurs, we can make assertions + * knowing that all R/W files should have been deleted and all + * R/O files rejected. + * <p> + * Value: {@value}. + */ + public static final int FILE_COUNT_SCALED = 10; + + public static final int DIR_COUNT = 2; + public static final int DIR_COUNT_SCALED = 4; + public static final int DEPTH = 2; + public static final int DEPTH_SCALED = 2; + + /** + * A role FS; if non-null it is closed in teardown. + */ + private S3AFileSystem roleFS; + + /** + * Base path for this test run. + * This is generated uniquely for each test. + */ + private Path basePath; + + /** + * A directory which restricted roles have full write access to. + */ + private Path writableDir; + + /** + * A directory to which restricted roles have only read access. + */ + private Path readOnlyDir; + + /** + * A file under {@link #readOnlyDir} which cannot be written or deleted. + */ + private Path readOnlyChild; + + /** + * A directory to which restricted roles have no read access. + */ + private Path noReadDir; + + /** delete policy: single or multi? */ + private final boolean multiDelete; + + /** + * Configuration for the assume role FS. + */ + private Configuration assumedRoleConfig; + + private int fileCount; + private int dirCount; + private int dirDepth; + + /** + * Was the -Dscale switch passed in to the test run? + */ + private boolean scaleTest; + + /** + * Test array for parameterized test runs. + * <ul> + * <li>Run 0: single deletes</li> + * <li>Run 1: multi deletes</li> + * </ul> + * + * @return a list of parameter tuples. + */ + @Parameterized.Parameters(name = "bulk-delete={0}") + public static Collection<Object[]> params() { + return Arrays.asList(new Object[][]{ + {false}, + {true}, + }); + } + + /** + * Constructor. + * @param multiDelete single vs multi delete in the role FS? + */ + public ITestPartialRenamesDeletes(final boolean multiDelete) { + this.multiDelete = multiDelete; + } + + /** + * This sets up a unique path for every test run, so as to guarantee isolation + * from previous runs. + * It creates a role policy which has read access to everything except + * the contents of {@link #noReadDir}, and with write access to + * {@link #writableDir}. + * @throws Exception failure + */ + @Override + public void setup() throws Exception { + super.setup(); + assumeRoleTests(); + basePath = uniquePath(); + readOnlyDir = new Path(basePath, "readonlyDir"); + writableDir = new Path(basePath, "writableDir"); + readOnlyChild = new Path(readOnlyDir, "child"); + noReadDir = new Path(basePath, "noReadDir"); + // the full FS + S3AFileSystem fs = getFileSystem(); + fs.delete(basePath, true); + fs.mkdirs(writableDir); + + // create the baseline assumed role + assumedRoleConfig = createAssumedRoleConfig(); + bindRolePolicyStatements(assumedRoleConfig, + STATEMENT_S3GUARD_CLIENT, + STATEMENT_ALL_BUCKET_READ_ACCESS, // root: r-x + new Statement(Effects.Allow) // dest: rwx + .addActions(S3_PATH_RW_OPERATIONS) + .addResources(directory(writableDir)), + new Statement(Effects.Deny) // noReadDir: --x + .addActions(S3_ALL_GET) + .addActions(S3_ALL_PUT) + .addActions(S3_ALL_DELETE) + .addResources(directory(noReadDir))); + // the role configured to that set of restrictions + roleFS = (S3AFileSystem) readOnlyDir.getFileSystem(assumedRoleConfig); + + // switch to the big set of files iff this is a multidelete run + // with -Dscale set. + // without that the DELETE calls become a key part of the bottleneck + scaleTest = multiDelete && getTestPropertyBool( + getConfiguration(), + KEY_SCALE_TESTS_ENABLED, + DEFAULT_SCALE_TESTS_ENABLED); + fileCount = scaleTest ? FILE_COUNT_SCALED : FILE_COUNT_NON_SCALED; + dirCount = scaleTest ? DIR_COUNT_SCALED : DIR_COUNT; + dirDepth = scaleTest ? DEPTH_SCALED : DEPTH; + } + + @Override + public void teardown() throws Exception { + S3AUtils.closeAll(LOG, roleFS); + super.teardown(); + } + + /** + * Directory cleanup includes pruning everything under the path. + * This ensures that any in the tree from failed tests don't fill up + * the store with many, many, deleted entries. + * @throws IOException failure. + */ + @Override + protected void deleteTestDirInTeardown() throws IOException { + super.deleteTestDirInTeardown(); + Path path = getContract().getTestPath(); + try { + prune(path); + } catch (IOException e) { + LOG.warn("When pruning the test directory {}", path, e); + } + } + + private void assumeRoleTests() { + assume("No ARN for role tests", !getAssumedRoleARN().isEmpty()); + } + + private String getAssumedRoleARN() { + return getContract().getConf().getTrimmed(ASSUMED_ROLE_ARN, ""); + } + + /** + * Create the assumed role configuration. + * @return a config bonded to the ARN of the assumed role + */ + public Configuration createAssumedRoleConfig() { + return createAssumedRoleConfig(getAssumedRoleARN()); + } + + /** + * Create a config for an assumed role; it also disables FS caching + * and sets the multi delete option to that of the current mode. + * @param roleARN ARN of role + * @return the new configuration + */ + private Configuration createAssumedRoleConfig(String roleARN) { + Configuration conf = newAssumedRoleConfig(getContract().getConf(), + roleARN); + String bucketName = getTestBucketName(conf); + + removeBucketOverrides(bucketName, conf, ENABLE_MULTI_DELETE); + conf.setBoolean(ENABLE_MULTI_DELETE, multiDelete); + return conf; + } + + @Override + protected Configuration createConfiguration() { + Configuration conf = super.createConfiguration(); + String bucketName = getTestBucketName(conf); + + // ramp up the number of connections we can have for maximum PUT + // performance + removeBucketOverrides(bucketName, conf, + MAX_THREADS, + MAXIMUM_CONNECTIONS, + S3GUARD_DDB_BACKGROUND_SLEEP_MSEC_KEY); + conf.setInt(MAX_THREADS, EXECUTOR_THREAD_COUNT); + conf.setInt(MAXIMUM_CONNECTIONS, EXECUTOR_THREAD_COUNT * 2); + // turn off prune delays, so as to stop scale tests creating + // so much cruft that future CLI prune commands take forever + conf.setInt(S3GUARD_DDB_BACKGROUND_SLEEP_MSEC_KEY, 0); + return conf; + } + + /** + * Create a unique path, which includes method name, + * multidelete flag and a random UUID. + * @return a string to use for paths. + * @throws IOException path creation failure. + */ + private Path uniquePath() throws IOException { + return path( + String.format("%s-%s-%04d", + getMethodName(), + multiDelete ? "multi" : "single", + System.currentTimeMillis() % 10000)); + } + + /** + * This is here to verify role and path setup. + */ + @Test + public void testCannotTouchUnderRODir() throws Throwable { + forbidden("touching the empty child " + readOnlyChild, + "", + () -> { + touch(roleFS, readOnlyChild); + return readOnlyChild; + }); + } + @Test + public void testCannotReadUnderNoReadDir() throws Throwable { + Path path = new Path(noReadDir, "unreadable.txt"); + createFile(getFileSystem(), path, true, "readonly".getBytes()); + forbidden("trying to read " + path, + "", + () -> readUTF8(roleFS, path, -1)); + } + + @Test + public void testMultiDeleteOptionPropagated() throws Throwable { + describe("Verify the test parameter propagates to the store context"); + StoreContext ctx = roleFS.createStoreContext(); + Assertions.assertThat(ctx.isMultiObjectDeleteEnabled()) + .as(ctx.toString()) + .isEqualTo(multiDelete); + } + + /** + * Execute a sequence of rename operations with access locked down. + */ + @Test + public void testRenameParentPathNotWriteable() throws Throwable { + describe("rename with parent paths not writeable; multi=%s", multiDelete); + final Configuration conf = createAssumedRoleConfig(); + bindRolePolicyStatements(conf, + STATEMENT_S3GUARD_CLIENT, + STATEMENT_ALLOW_SSE_KMS_RW, + STATEMENT_ALL_BUCKET_READ_ACCESS, + new Statement(Effects.Allow) + .addActions(S3_PATH_RW_OPERATIONS) + .addResources(directory(readOnlyDir)) + .addResources(directory(writableDir))); + roleFS = (S3AFileSystem) readOnlyDir.getFileSystem(conf); + + S3AFileSystem fs = getFileSystem(); + roleFS.getFileStatus(ROOT); + fs.mkdirs(readOnlyDir); + // you can create an adjacent child + touch(fs, readOnlyChild); + + fs.delete(writableDir, true); + // as dest doesn't exist, this will map child -> dest + assertRenameOutcome(roleFS, readOnlyChild, writableDir, true); + + assertIsFile(writableDir); + assertIsDirectory(readOnlyDir); + Path renamedDestPath = new Path(readOnlyDir, writableDir.getName()); + assertRenameOutcome(roleFS, writableDir, readOnlyDir, true); + assertIsFile(renamedDestPath); Review comment: no, didn't actually. good point though ---------------------------------------------------------------- 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
