diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/GcTimeMonitor.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/GcTimeMonitor.java index 4247eb7050b..0640fc01e2f 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/GcTimeMonitor.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/GcTimeMonitor.java @@ -23,7 +23,6 @@ import java.lang.management.GarbageCollectorMXBean; import java.lang.management.ManagementFactory; import java.util.List; -import java.util.concurrent.TimeUnit; /** * This class monitors the percentage of time the JVM is paused in GC within @@ -47,52 +46,6 @@ private final GcData curData = new GcData(); private volatile boolean shouldRun = true; - public static class Builder { - - private long observationWindowMs = TimeUnit.MINUTES.toMillis(1); - private long sleepIntervalMs = TimeUnit.SECONDS.toMillis(5); - private int maxGcTimePercentage = 100; - private GcTimeAlertHandler handler = null; - - /** - * Set observation window size in milliseconds. - */ - public Builder observationWindowMs(long value) { - this.observationWindowMs = value; - return this; - } - - /** - * Set sleep interval in milliseconds. - */ - public Builder sleepIntervalMs(long value) { - this.sleepIntervalMs = value; - return this; - } - - /** - * Set the max GC time percentage that triggers the alert handler. - */ - public Builder maxGcTimePercentage(int value) { - this.maxGcTimePercentage = value; - return this; - } - - /** - * Set the GC alert handler. - */ - public Builder gcTimeAlertHandler(GcTimeAlertHandler value) { - this.handler = value; - return this; - } - - public GcTimeMonitor build() { - return new GcTimeMonitor(observationWindowMs, sleepIntervalMs, - maxGcTimePercentage, handler); - } - } - - /** * Create an instance of GCTimeMonitor. Once it's started, it will stay alive * and monitor GC time percentage until shutdown() is called. If you don't diff --git a/hadoop-common-project/hadoop-common/src/site/markdown/Metrics.md b/hadoop-common-project/hadoop-common/src/site/markdown/Metrics.md index bafdfddf16b..2d0f23293bf 100644 --- a/hadoop-common-project/hadoop-common/src/site/markdown/Metrics.md +++ b/hadoop-common-project/hadoop-common/src/site/markdown/Metrics.md @@ -56,7 +56,6 @@ Each metrics record contains tags such as ProcessName, SessionID and Hostname as | `GcNumWarnThresholdExceeded` | Number of times that the GC warn threshold is exceeded | | `GcNumInfoThresholdExceeded` | Number of times that the GC info threshold is exceeded | | `GcTotalExtraSleepTime` | Total GC extra sleep time in msec | -| `GcTimePercentage` | The percentage (0..100) of time that the JVM spent in GC pauses within the observation window if `dfs.namenode.gc.time.monitor.enable` is set to true. Use `dfs.namenode.gc.time.monitor.sleep.interval.ms` to specify the sleep interval in msec. Use `dfs.namenode.gc.time.monitor.observation.window.ms` to specify the observation window in msec. | rpc context =========== diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractUnbufferTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractUnbufferTest.java index 5eb1e892f83..7ba32bafa55 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractUnbufferTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractUnbufferTest.java @@ -18,13 +18,12 @@ package org.apache.hadoop.fs.contract; +import org.apache.hadoop.fs.FSDataInputStream; +import org.apache.hadoop.fs.Path; + import org.junit.Test; import java.io.IOException; -import java.util.Arrays; - -import org.apache.hadoop.fs.FSDataInputStream; -import org.apache.hadoop.fs.Path; import static org.apache.hadoop.fs.contract.ContractTestUtils.createFile; import static org.apache.hadoop.fs.contract.ContractTestUtils.dataset; @@ -35,22 +34,21 @@ public abstract class AbstractContractUnbufferTest extends AbstractFSContractTestBase { private Path file; - private byte[] fileBytes; @Override public void setup() throws Exception { super.setup(); skipIfUnsupported(SUPPORTS_UNBUFFER); file = path("unbufferFile"); - fileBytes = dataset(TEST_FILE_LEN, 0, 255); - createFile(getFileSystem(), file, true, fileBytes); + createFile(getFileSystem(), file, true, + dataset(TEST_FILE_LEN, 0, 255)); } @Test public void testUnbufferAfterRead() throws IOException { describe("unbuffer a file after a single read"); try (FSDataInputStream stream = getFileSystem().open(file)) { - validateFullFileContents(stream); + assertEquals(128, stream.read(new byte[128])); unbuffer(stream); } } @@ -60,14 +58,15 @@ public void testUnbufferBeforeRead() throws IOException { describe("unbuffer a file before a read"); try (FSDataInputStream stream = getFileSystem().open(file)) { unbuffer(stream); - validateFullFileContents(stream); + assertEquals(128, stream.read(new byte[128])); } } @Test public void testUnbufferEmptyFile() throws IOException { Path emptyFile = path("emptyUnbufferFile"); - getFileSystem().create(emptyFile, true).close(); + createFile(getFileSystem(), emptyFile, true, + dataset(TEST_FILE_LEN, 0, 255)); describe("unbuffer an empty file"); try (FSDataInputStream stream = getFileSystem().open(emptyFile)) { unbuffer(stream); @@ -80,15 +79,13 @@ public void testUnbufferOnClosedFile() throws IOException { FSDataInputStream stream = null; try { stream = getFileSystem().open(file); - validateFullFileContents(stream); + assertEquals(128, stream.read(new byte[128])); } finally { if (stream != null) { stream.close(); } } - if (stream != null) { - unbuffer(stream); - } + unbuffer(stream); } @Test @@ -97,58 +94,32 @@ public void testMultipleUnbuffers() throws IOException { try (FSDataInputStream stream = getFileSystem().open(file)) { unbuffer(stream); unbuffer(stream); - validateFullFileContents(stream); + assertEquals(128, stream.read(new byte[128])); unbuffer(stream); unbuffer(stream); } } - @Test + @Test public void testUnbufferMultipleReads() throws IOException { describe("unbuffer a file multiple times"); try (FSDataInputStream stream = getFileSystem().open(file)) { unbuffer(stream); - validateFileContents(stream, TEST_FILE_LEN / 8, 0); + assertEquals(128, stream.read(new byte[128])); unbuffer(stream); - validateFileContents(stream, TEST_FILE_LEN / 8, TEST_FILE_LEN / 8); - validateFileContents(stream, TEST_FILE_LEN / 4, TEST_FILE_LEN / 4); + assertEquals(128, stream.read(new byte[128])); + assertEquals(128, stream.read(new byte[128])); unbuffer(stream); - validateFileContents(stream, TEST_FILE_LEN / 2, TEST_FILE_LEN / 2); + assertEquals(128, stream.read(new byte[128])); + assertEquals(128, stream.read(new byte[128])); + assertEquals(128, stream.read(new byte[128])); unbuffer(stream); - assertEquals("stream should be at end of file", TEST_FILE_LEN, - stream.getPos()); } } private void unbuffer(FSDataInputStream stream) throws IOException { long pos = stream.getPos(); stream.unbuffer(); - assertEquals("unbuffer unexpectedly changed the stream position", pos, - stream.getPos()); - } - - protected void validateFullFileContents(FSDataInputStream stream) - throws IOException { - validateFileContents(stream, TEST_FILE_LEN, 0); - } - - protected void validateFileContents(FSDataInputStream stream, int length, - int startIndex) - throws IOException { - byte[] streamData = new byte[length]; - assertEquals("failed to read expected number of bytes from " - + "stream", length, stream.read(streamData)); - byte[] validateFileBytes; - if (startIndex == 0 && length == fileBytes.length) { - validateFileBytes = fileBytes; - } else { - validateFileBytes = Arrays.copyOfRange(fileBytes, startIndex, - startIndex + length); - } - assertArrayEquals("invalid file contents", validateFileBytes, streamData); - } - - protected Path getFile() { - return file; + assertEquals(pos, stream.getPos()); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSUtilClient.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSUtilClient.java index 95aad12d928..795ddfb379f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSUtilClient.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSUtilClient.java @@ -35,7 +35,6 @@ import org.apache.hadoop.hdfs.protocol.ClientDatanodeProtocol; import org.apache.hadoop.hdfs.protocol.DatanodeID; import org.apache.hadoop.hdfs.protocol.DatanodeInfo; -import org.apache.hadoop.hdfs.protocol.EncryptionZone; import org.apache.hadoop.hdfs.protocol.ExtendedBlock; import org.apache.hadoop.hdfs.protocol.HdfsConstants; import org.apache.hadoop.hdfs.protocol.LocatedBlock; @@ -1004,7 +1003,7 @@ public static Path makePathFromFileId(long fileId) { * @param ugi {@link UserGroupInformation} of current user. * @return the home directory of current user. */ - public static String getHomeDirectory(Configuration conf, + public static Path getHomeDirectory(Configuration conf, UserGroupInformation ugi) { String userHomePrefix = HdfsClientConfigKeys .DFS_USER_HOME_DIR_PREFIX_DEFAULT; @@ -1013,31 +1012,6 @@ public static String getHomeDirectory(Configuration conf, HdfsClientConfigKeys.DFS_USER_HOME_DIR_PREFIX_KEY, HdfsClientConfigKeys.DFS_USER_HOME_DIR_PREFIX_DEFAULT); } - return userHomePrefix + Path.SEPARATOR + ugi.getShortUserName(); - } - - /** - * Returns trash root in non-encryption zone. - * @param conf configuration. - * @param ugi user of trash owner. - * @return unqualified path of trash root. - */ - public static String getTrashRoot(Configuration conf, - UserGroupInformation ugi) { - return getHomeDirectory(conf, ugi) - + Path.SEPARATOR + FileSystem.TRASH_PREFIX; - } - - /** - * Returns trash root in encryption zone. - * @param ez encryption zone. - * @param ugi user of trash owner. - * @return unqualified path of trash root. - */ - public static String getEZTrashRoot(EncryptionZone ez, - UserGroupInformation ugi) { - String ezpath = ez.getPath(); - return (ezpath.equals("/") ? ezpath : ezpath + Path.SEPARATOR) - + FileSystem.TRASH_PREFIX + Path.SEPARATOR + ugi.getShortUserName(); + return new Path(userHomePrefix + "/" + ugi.getShortUserName()); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DeadNodeDetector.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DeadNodeDetector.java index 75a91ba7188..ce5054761a7 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DeadNodeDetector.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DeadNodeDetector.java @@ -24,6 +24,7 @@ import org.apache.hadoop.hdfs.protocol.DatanodeLocalInfo; import org.apache.hadoop.hdfs.protocol.HdfsConstants; import org.apache.hadoop.util.Daemon; +import org.apache.hadoop.util.Time; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -103,6 +104,11 @@ private Map probeInProg = new ConcurrentHashMap(); + /** + * The last time when detect dead node. + */ + private long lastDetectDeadTS = 0; + /** * Interval time in milliseconds for probing dead node behavior. */ @@ -410,15 +416,20 @@ private void probeCallBack(Probe probe, boolean success) { * Check dead node periodically. */ private void checkDeadNodes() { - Set datanodeInfos = clearAndGetDetectedDeadNodes(); - for (DatanodeInfo datanodeInfo : datanodeInfos) { - LOG.debug("Add dead node to check: {}.", datanodeInfo); - if (!deadNodesProbeQueue.offer(datanodeInfo)) { - LOG.debug("Skip to add dead node {} to check " + - "since the probe queue is full.", datanodeInfo); - break; + long ts = Time.monotonicNow(); + if (ts - lastDetectDeadTS > deadNodeDetectInterval) { + Set datanodeInfos = clearAndGetDetectedDeadNodes(); + for (DatanodeInfo datanodeInfo : datanodeInfos) { + LOG.debug("Add dead node to check: {}.", datanodeInfo); + if (!deadNodesProbeQueue.offer(datanodeInfo)) { + LOG.debug("Skip to add dead node {} to check " + + "since the probe queue is full.", datanodeInfo); + break; + } } + lastDetectDeadTS = ts; } + state = State.IDLE; } diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java index 9628f4c9a8b..41f176318a9 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java @@ -220,8 +220,7 @@ public void setWorkingDirectory(Path dir) { @Override public Path getHomeDirectory() { - return makeQualified( - new Path(DFSUtilClient.getHomeDirectory(getConf(), dfs.ugi))); + return makeQualified(DFSUtilClient.getHomeDirectory(getConf(), dfs.ugi)); } /** @@ -3237,7 +3236,8 @@ public Path getTrashRoot(Path path) { EncryptionZone ez = dfs.getEZForPath(parentSrc); if ((ez != null)) { return this.makeQualified( - new Path(DFSUtilClient.getEZTrashRoot(ez, dfs.ugi))); + new Path(new Path(ez.getPath(), FileSystem.TRASH_PREFIX), + dfs.ugi.getShortUserName())); } } catch (IOException e) { DFSClient.LOG.warn("Exception in checking the encryption zone for the " + @@ -3264,8 +3264,7 @@ public Path getTrashRoot(Path path) { // Get EZ Trash roots final RemoteIterator it = dfs.listEncryptionZones(); while (it.hasNext()) { - EncryptionZone ez = it.next(); - Path ezTrashRoot = new Path(ez.getPath(), + Path ezTrashRoot = new Path(it.next().getPath(), FileSystem.TRASH_PREFIX); if (!exists(ezTrashRoot)) { continue; @@ -3277,7 +3276,7 @@ public Path getTrashRoot(Path path) { } } } else { - Path userTrash = new Path(DFSUtilClient.getEZTrashRoot(ez, dfs.ugi)); + Path userTrash = new Path(ezTrashRoot, dfs.ugi.getShortUserName()); try { ret.add(getFileStatus(userTrash)); } catch (FileNotFoundException ignored) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java index bb8039c2d93..f0f8f3e1518 100755 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java @@ -290,10 +290,6 @@ = "dfs.namenode.blockreport.queue.size"; public static final int DFS_NAMENODE_BLOCKREPORT_QUEUE_SIZE_DEFAULT = 1024; - public static final String DFS_NAMENODE_BLOCKREPORT_MAX_LOCK_HOLD_TIME - = "dfs.namenode.blockreport.max.lock.hold.time"; - public static final long - DFS_NAMENODE_BLOCKREPORT_MAX_LOCK_HOLD_TIME_DEFAULT = 4; @Deprecated public static final String DFS_WEBHDFS_USER_PATTERN_KEY = HdfsClientConfigKeys.DFS_WEBHDFS_USER_PATTERN_KEY; @@ -1069,21 +1065,6 @@ public static final String DFS_NAMENODE_BLOCKPLACEMENTPOLICY_DEFAULT_PREFER_LOCAL_NODE_KEY = "dfs.namenode.block-placement-policy.default.prefer-local-node"; public static final boolean DFS_NAMENODE_BLOCKPLACEMENTPOLICY_DEFAULT_PREFER_LOCAL_NODE_DEFAULT = true; - public static final String DFS_NAMENODE_GC_TIME_MONITOR_ENABLE = - "dfs.namenode.gc.time.monitor.enable"; - public static final boolean DFS_NAMENODE_GC_TIME_MONITOR_ENABLE_DEFAULT = - true; - public static final String - DFS_NAMENODE_GC_TIME_MONITOR_OBSERVATION_WINDOW_MS = - "dfs.namenode.gc.time.monitor.observation.window.ms"; - public static final long - DFS_NAMENODE_GC_TIME_MONITOR_OBSERVATION_WINDOW_MS_DEFAULT = - TimeUnit.MINUTES.toMillis(1); - public static final String DFS_NAMENODE_GC_TIME_MONITOR_SLEEP_INTERVAL_MS = - "dfs.namenode.gc.time.monitor.sleep.interval.ms"; - public static final long - DFS_NAMENODE_GC_TIME_MONITOR_SLEEP_INTERVAL_MS_DEFAULT = - TimeUnit.SECONDS.toMillis(5); public static final String DFS_BLOCK_LOCAL_PATH_ACCESS_USER_KEY = "dfs.block.local-path-access.user"; public static final String DFS_DOMAIN_SOCKET_PATH_KEY = diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java index 626048f1cf7..45d3b792497 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java @@ -393,9 +393,6 @@ public long getTotalECBlockGroups() { // Max number of blocks to log info about during a block report. private final long maxNumBlocksToLog; - // Max write lock hold time for BlockReportProcessingThread(ms). - private final long maxLockHoldTime; - /** * When running inside a Standby node, the node may receive block reports * from datanodes before receiving the corresponding namespace edits from @@ -546,10 +543,6 @@ public BlockManager(final Namesystem namesystem, boolean haEnabled, this.maxNumBlocksToLog = conf.getLong(DFSConfigKeys.DFS_MAX_NUM_BLOCKS_TO_LOG_KEY, DFSConfigKeys.DFS_MAX_NUM_BLOCKS_TO_LOG_DEFAULT); - this.maxLockHoldTime = conf.getTimeDuration( - DFSConfigKeys.DFS_NAMENODE_BLOCKREPORT_MAX_LOCK_HOLD_TIME, - DFSConfigKeys.DFS_NAMENODE_BLOCKREPORT_MAX_LOCK_HOLD_TIME_DEFAULT, - TimeUnit.MILLISECONDS); this.numBlocksPerIteration = conf.getInt( DFSConfigKeys.DFS_BLOCK_MISREPLICATION_PROCESSING_LIMIT, DFSConfigKeys.DFS_BLOCK_MISREPLICATION_PROCESSING_LIMIT_DEFAULT); @@ -3142,7 +3135,6 @@ public void processQueuedMessagesForBlock(Block b) throws IOException { private void processQueuedMessages(Iterable rbis) throws IOException { - boolean isPreviousMessageProcessed = true; for (ReportedBlockInfo rbi : rbis) { LOG.debug("Processing previouly queued message {}", rbi); if (rbi.getReportedState() == null) { @@ -3150,15 +3142,9 @@ private void processQueuedMessages(Iterable rbis) DatanodeStorageInfo storageInfo = rbi.getStorageInfo(); removeStoredBlock(getStoredBlock(rbi.getBlock()), storageInfo.getDatanodeDescriptor()); - } else if (!isPreviousMessageProcessed) { - // if the previous IBR processing was skipped, skip processing all - // further IBR's so as to ensure same sequence of processing. - queueReportedBlock(rbi.getStorageInfo(), rbi.getBlock(), - rbi.getReportedState(), QUEUE_REASON_FUTURE_GENSTAMP); } else { - isPreviousMessageProcessed = - processAndHandleReportedBlock(rbi.getStorageInfo(), rbi.getBlock(), - rbi.getReportedState(), null); + processAndHandleReportedBlock(rbi.getStorageInfo(), + rbi.getBlock(), rbi.getReportedState(), null); } } } @@ -4105,14 +4091,8 @@ public void addBlock(DatanodeStorageInfo storageInfo, Block block, processAndHandleReportedBlock(storageInfo, block, ReplicaState.FINALIZED, delHintNode); } - - /** - * Process a reported block. - * @return true if the block is processed, or false if the block is queued - * to be processed later. - * @throws IOException - */ - private boolean processAndHandleReportedBlock( + + private void processAndHandleReportedBlock( DatanodeStorageInfo storageInfo, Block block, ReplicaState reportedState, DatanodeDescriptor delHintNode) throws IOException { @@ -4126,7 +4106,7 @@ private boolean processAndHandleReportedBlock( isGenStampInFuture(block)) { queueReportedBlock(storageInfo, block, reportedState, QUEUE_REASON_FUTURE_GENSTAMP); - return false; + return; } // find block by blockId @@ -4137,7 +4117,7 @@ private boolean processAndHandleReportedBlock( blockLog.debug("BLOCK* addBlock: block {} on node {} size {} does not " + "belong to any file", block, node, block.getNumBytes()); addToInvalidates(new Block(block), node); - return true; + return; } BlockUCState ucState = storedBlock.getBlockUCState(); @@ -4146,7 +4126,7 @@ private boolean processAndHandleReportedBlock( // Ignore replicas already scheduled to be removed from the DN if(invalidateBlocks.contains(node, block)) { - return true; + return; } BlockToMarkCorrupt c = checkReplicaCorrupt( @@ -4164,14 +4144,14 @@ private boolean processAndHandleReportedBlock( } else { markBlockAsCorrupt(c, storageInfo, node); } - return true; + return; } if (isBlockUnderConstruction(storedBlock, ucState, reportedState)) { addStoredBlockUnderConstruction( new StatefulBlockInfo(storedBlock, new Block(block), reportedState), storageInfo); - return true; + return; } // Add replica if appropriate. If the replica was previously corrupt @@ -4181,7 +4161,6 @@ private boolean processAndHandleReportedBlock( corruptReplicas.isReplicaCorrupt(storedBlock, node))) { addStoredBlock(storedBlock, block, storageInfo, delHintNode, true); } - return true; } /** @@ -5180,6 +5159,7 @@ public int getBlockOpQueueLength() { } private class BlockReportProcessingThread extends Thread { + private static final long MAX_LOCK_HOLD_MS = 4; private long lastFull = 0; private final BlockingQueue queue; @@ -5215,7 +5195,7 @@ private void processQueue() { do { processed++; action.run(); - if (Time.monotonicNow() - start > maxLockHoldTime) { + if (Time.monotonicNow() - start > MAX_LOCK_HOLD_MS) { break; } action = queue.poll(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/ReplicaCachingGetSpaceUsed.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/ReplicaCachingGetSpaceUsed.java index 5acc3c04279..d2dc90ec526 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/ReplicaCachingGetSpaceUsed.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/ReplicaCachingGetSpaceUsed.java @@ -86,7 +86,7 @@ protected void refresh() { for (ReplicaInfo replicaInfo : replicaInfos) { if (Objects.equals(replicaInfo.getVolume().getStorageID(), volume.getStorageID())) { - dfsUsed += replicaInfo.getBytesOnDisk(); + dfsUsed += replicaInfo.getBlockDataLength(); dfsUsed += replicaInfo.getMetadataLength(); count++; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/BackupImage.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/BackupImage.java index 9f5f29e3714..0737824d4ee 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/BackupImage.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/BackupImage.java @@ -218,12 +218,7 @@ private synchronized void applyEdits(long firstTxId, int numTxns, byte[] data) } lastAppliedTxId = logLoader.getLastAppliedTxId(); - getNamesystem().writeLock(); - try { - getNamesystem().dir.updateCountForQuota(); - } finally { - getNamesystem().writeUnlock(); - } + getNamesystem().dir.updateCountForQuota(); } finally { backupInputStream.clear(); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EditLogFileInputStream.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EditLogFileInputStream.java index 26c5e9049f7..b81cd1cf97a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EditLogFileInputStream.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EditLogFileInputStream.java @@ -67,6 +67,7 @@ CLOSED } private State state = State.UNINIT; + private InputStream fStream = null; private int logVersion = 0; private FSEditLogOp.Reader reader = null; private FSEditLogLoader.PositionTrackingInputStream tracker = null; @@ -152,7 +153,6 @@ private void init(boolean verifyLayoutVersion) throws LogHeaderCorruptException, IOException { Preconditions.checkState(state == State.UNINIT); BufferedInputStream bin = null; - InputStream fStream = null; try { fStream = log.getInputStream(); bin = new BufferedInputStream(fStream); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java index 48e405ba354..e98d1126ffc 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java @@ -187,11 +187,11 @@ public void pauseForTestingAfterNthCheckpoint(final String zone, final int count) throws IOException { INodesInPath iip; final FSPermissionChecker pc = dir.getPermissionChecker(); - dir.getFSNamesystem().readLock(); + dir.readLock(); try { iip = dir.resolvePath(pc, zone, DirOp.READ); } finally { - dir.getFSNamesystem().readUnlock(); + dir.readUnlock(); } reencryptionHandler .pauseForTestingAfterNthCheckpoint(iip.getLastINode().getId(), count); @@ -280,11 +280,11 @@ void stopReencryptThread() { if (getProvider() == null || reencryptionHandler == null) { return; } - dir.getFSNamesystem().writeLock(); + dir.writeLock(); try { reencryptionHandler.stopThreads(); } finally { - dir.getFSNamesystem().writeUnlock(); + dir.writeUnlock(); } if (reencryptHandlerExecutor != null) { reencryptHandlerExecutor.shutdownNow(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java index 6c7b1fae50d..02e09faff46 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java @@ -382,6 +382,7 @@ private static ZoneEncryptionInfoProto getZoneEncryptionInfoProto( static void saveFileXAttrsForBatch(FSDirectory fsd, List batch) { assert fsd.getFSNamesystem().hasWriteLock(); + assert !fsd.hasWriteLock(); if (batch != null && !batch.isEmpty()) { for (FileEdekInfo entry : batch) { final INode inode = fsd.getInode(entry.getInodeId()); @@ -726,13 +727,13 @@ static String getKeyNameForZone(final FSDirectory dir, final FSPermissionChecker pc, final String zone) throws IOException { assert dir.getProvider() != null; final INodesInPath iip; - dir.getFSNamesystem().readLock(); + dir.readLock(); try { iip = dir.resolvePath(pc, zone, DirOp.READ); dir.ezManager.checkEncryptionZoneRoot(iip.getLastINode(), zone); return dir.ezManager.getKeyName(iip); } finally { - dir.getFSNamesystem().readUnlock(); + dir.readUnlock(); } } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java index 77d8518de91..a926afe13ff 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java @@ -82,6 +82,7 @@ import java.util.TreeSet; import java.util.concurrent.ForkJoinPool; import java.util.concurrent.RecursiveAction; +import java.util.concurrent.locks.ReentrantReadWriteLock; import static org.apache.hadoop.fs.CommonConfigurationKeys.FS_PROTECTED_DIRECTORIES; import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_ACCESSTIME_PRECISION_DEFAULT; @@ -171,6 +172,9 @@ private static INodeDirectory createRoot(FSNamesystem namesystem) { // Each entry in this set must be a normalized path. private volatile SortedSet protectedDirectories; + // lock to protect the directory and BlockMap + private final ReentrantReadWriteLock dirLock; + private final boolean isPermissionEnabled; private final boolean isPermissionContentSummarySubAccess; /** @@ -211,44 +215,37 @@ public void setINodeAttributeProvider(INodeAttributeProvider provider) { attributeProvider = provider; } - /** - * The directory lock dirLock provided redundant locking. - * It has been used whenever namesystem.fsLock was used. - * dirLock is now removed and utility methods to acquire and release dirLock - * remain as placeholders only - */ + // utility methods to acquire and release read lock and write lock void readLock() { - assert namesystem.hasReadLock() : "Should hold namesystem read lock"; + this.dirLock.readLock().lock(); } void readUnlock() { - assert namesystem.hasReadLock() : "Should hold namesystem read lock"; + this.dirLock.readLock().unlock(); } void writeLock() { - assert namesystem.hasWriteLock() : "Should hold namesystem write lock"; + this.dirLock.writeLock().lock(); } void writeUnlock() { - assert namesystem.hasWriteLock() : "Should hold namesystem write lock"; + this.dirLock.writeLock().unlock(); } boolean hasWriteLock() { - return namesystem.hasWriteLock(); + return this.dirLock.isWriteLockedByCurrentThread(); } boolean hasReadLock() { - return namesystem.hasReadLock(); + return this.dirLock.getReadHoldCount() > 0 || hasWriteLock(); } - @Deprecated // dirLock is obsolete, use namesystem.fsLock instead public int getReadHoldCount() { - return namesystem.getReadHoldCount(); + return this.dirLock.getReadHoldCount(); } - @Deprecated // dirLock is obsolete, use namesystem.fsLock instead public int getWriteHoldCount() { - return namesystem.getWriteHoldCount(); + return this.dirLock.getWriteHoldCount(); } public int getListLimit() { @@ -276,6 +273,7 @@ public int getListLimit() { }; FSDirectory(FSNamesystem ns, Configuration conf) throws IOException { + this.dirLock = new ReentrantReadWriteLock(true); // fair this.inodeId = new INodeId(); rootDir = createRoot(ns); inodeMap = INodeMap.newInstance(rootDir); @@ -1494,7 +1492,12 @@ public final void removeFromInodeMap(List inodes) { * @return The inode associated with the given id */ public INode getInode(long id) { - return inodeMap.get(id); + readLock(); + try { + return inodeMap.get(id); + } finally { + readUnlock(); + } } @VisibleForTesting diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index 004da05fe71..ab12c8880fb 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -3723,7 +3723,6 @@ INodeFile getBlockCollection(BlockInfo b) { @Override public INodeFile getBlockCollection(long id) { - assert hasReadLock() : "Accessing INode id = " + id + " without read lock"; INode inode = getFSDirectory().getInode(id); return inode == null ? null : inode.asFile(); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java index 2a74190995c..66c5de6c487 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java @@ -96,8 +96,6 @@ import org.apache.hadoop.util.ServicePlugin; import org.apache.hadoop.util.StringUtils; import org.apache.hadoop.util.Time; -import org.apache.hadoop.util.GcTimeMonitor; -import org.apache.hadoop.util.GcTimeMonitor.Builder; import org.apache.htrace.core.Tracer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -178,12 +176,6 @@ import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_REPLICATION_STREAMS_HARD_LIMIT_DEFAULT; import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_REPLICATION_WORK_MULTIPLIER_PER_ITERATION; import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_REPLICATION_WORK_MULTIPLIER_PER_ITERATION_DEFAULT; -import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_GC_TIME_MONITOR_SLEEP_INTERVAL_MS; -import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_GC_TIME_MONITOR_SLEEP_INTERVAL_MS_DEFAULT; -import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_GC_TIME_MONITOR_OBSERVATION_WINDOW_MS; -import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_GC_TIME_MONITOR_OBSERVATION_WINDOW_MS_DEFAULT; -import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_GC_TIME_MONITOR_ENABLE; -import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_GC_TIME_MONITOR_ENABLE_DEFAULT; import static org.apache.hadoop.util.ExitUtil.terminate; import static org.apache.hadoop.util.ToolRunner.confirmPrompt; @@ -419,7 +411,6 @@ public long getProtocolVersion(String protocol, private NameNodeRpcServer rpcServer; private JvmPauseMonitor pauseMonitor; - private GcTimeMonitor gcTimeMonitor; private ObjectName nameNodeStatusBeanName; protected final Tracer tracer; protected final TracerConfigurationManager tracerConfigurationManager; @@ -733,22 +724,6 @@ protected void initialize(Configuration conf) throws IOException { pauseMonitor.start(); metrics.getJvmMetrics().setPauseMonitor(pauseMonitor); - if (conf.getBoolean(DFS_NAMENODE_GC_TIME_MONITOR_ENABLE, - DFS_NAMENODE_GC_TIME_MONITOR_ENABLE_DEFAULT)) { - long observationWindow = conf.getTimeDuration( - DFS_NAMENODE_GC_TIME_MONITOR_OBSERVATION_WINDOW_MS, - DFS_NAMENODE_GC_TIME_MONITOR_OBSERVATION_WINDOW_MS_DEFAULT, - TimeUnit.MILLISECONDS); - long sleepInterval = conf.getTimeDuration( - DFS_NAMENODE_GC_TIME_MONITOR_SLEEP_INTERVAL_MS, - DFS_NAMENODE_GC_TIME_MONITOR_SLEEP_INTERVAL_MS_DEFAULT, - TimeUnit.MILLISECONDS); - gcTimeMonitor = new Builder().observationWindowMs(observationWindow) - .sleepIntervalMs(sleepInterval).build(); - gcTimeMonitor.start(); - metrics.getJvmMetrics().setGcTimeMonitor(gcTimeMonitor); - } - if (NamenodeRole.NAMENODE == role) { startHttpServer(conf); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ReencryptionHandler.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ReencryptionHandler.java index fd9cbd75275..2e13df5fd37 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ReencryptionHandler.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/ReencryptionHandler.java @@ -338,7 +338,7 @@ public void run() { } final Long zoneId; - dir.getFSNamesystem().readLock(); + dir.readLock(); try { zoneId = getReencryptionStatus().getNextUnprocessedZone(); if (zoneId == null) { @@ -350,7 +350,7 @@ public void run() { getReencryptionStatus().markZoneStarted(zoneId); resetSubmissionTracker(zoneId); } finally { - dir.getFSNamesystem().readUnlock(); + dir.readUnlock(); } try { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/startupprogress/StartupProgress.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/startupprogress/StartupProgress.java index 6249a84e7f9..b8244a156a4 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/startupprogress/StartupProgress.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/startupprogress/StartupProgress.java @@ -218,7 +218,7 @@ public void setSize(Phase phase, long size) { * @param total long to set */ public void setTotal(Phase phase, Step step, long total) { - if (!isComplete(phase)) { + if (!isComplete()) { lazyInitStep(phase, step).total = total; } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java index 2423a037c8f..78eba410bd8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/web/resources/NamenodeWebHdfsMethods.java @@ -65,6 +65,7 @@ import org.apache.hadoop.fs.ContentSummary; import org.apache.hadoop.fs.FileEncryptionInfo; import org.apache.hadoop.fs.FileStatus; +import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.FsServerDefaults; import org.apache.hadoop.fs.Options; import org.apache.hadoop.fs.XAttr; @@ -80,7 +81,6 @@ import org.apache.hadoop.hdfs.protocol.ClientProtocol; import org.apache.hadoop.hdfs.protocol.DatanodeInfo; import org.apache.hadoop.hdfs.protocol.DirectoryListing; -import org.apache.hadoop.hdfs.protocol.EncryptionZone; import org.apache.hadoop.hdfs.protocol.ErasureCodingPolicy; import org.apache.hadoop.hdfs.protocol.HdfsFileStatus; import org.apache.hadoop.hdfs.protocol.LocatedBlocks; @@ -1244,7 +1244,7 @@ protected Response get( return Response.ok(js).type(MediaType.APPLICATION_JSON).build(); } case GETHOMEDIRECTORY: { - String userHome = DFSUtilClient.getHomeDirectory(conf, ugi); + String userHome = DFSUtilClient.getHomeDirectory(conf, ugi).toString(); final String js = JsonUtil.toJsonString("Path", userHome); return Response.ok(js).type(MediaType.APPLICATION_JSON).build(); } @@ -1285,7 +1285,7 @@ protected Response get( return Response.ok().build(); } case GETTRASHROOT: { - final String trashPath = getTrashRoot(conf, fullpath); + final String trashPath = getTrashRoot(fullpath, conf); final String jsonStr = JsonUtil.toJsonString("Path", trashPath); return Response.ok(jsonStr).type(MediaType.APPLICATION_JSON).build(); } @@ -1345,39 +1345,11 @@ protected Response get( } } - private String getTrashRoot(Configuration conf, String fullPath) - throws IOException { - UserGroupInformation ugi= UserGroupInformation.getCurrentUser(); - String parentSrc = getParent(fullPath); - EncryptionZone ez = getRpcClientProtocol().getEZForPath( - parentSrc != null ? parentSrc : fullPath); - String trashRoot; - if (ez != null) { - trashRoot = DFSUtilClient.getEZTrashRoot(ez, ugi); - } else { - trashRoot = DFSUtilClient.getTrashRoot(conf, ugi); - } - return trashRoot; - } - - /** - * Returns the parent of a path in the same way as Path#getParent. - * @return the parent of a path or null if at root - */ - public String getParent(String path) { - int lastSlash = path.lastIndexOf('/'); - int start = 0; - if ((path.length() == start) || // empty path - (lastSlash == start && path.length() == start + 1)) { // at root - return null; - } - String parent; - if (lastSlash == -1) { - parent = org.apache.hadoop.fs.Path.CUR_DIR; - } else { - parent = path.substring(0, lastSlash == start ? start + 1 : lastSlash); - } - return parent; + private static String getTrashRoot(String fullPath, + Configuration conf) throws IOException { + FileSystem fs = FileSystem.get(conf != null ? conf : new Configuration()); + return fs.getTrashRoot( + new org.apache.hadoop.fs.Path(fullPath)).toUri().getPath(); } private static DirectoryListing getDirectoryListing(final ClientProtocol cp, diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml b/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml index ad556c60b6e..5daaffbe3ae 100755 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml @@ -5736,14 +5736,6 @@ - - dfs.namenode.blockreport.max.lock.hold.time - 4 - - The BlockReportProcessingThread max write lock hold time in ms. - - - dfs.journalnode.edits.dir.perm 700 @@ -5761,34 +5753,4 @@ Determines the namenode automatic lease recovery interval in seconds. - - - dfs.namenode.gc.time.monitor.enable - true - - Enable the GcTimePercentage metrics in NameNode's JvmMetrics. It will - start a thread(GcTimeMonitor) computing the metric. - - - - - dfs.namenode.gc.time.monitor.observation.window.ms - 1m - - Determines the windows size of GcTimeMonitor. A window is a period of time - starts at now-windowSize and ends at now. The GcTimePercentage is the gc - time proportion of the window. - - - - - dfs.namenode.gc.time.monitor.sleep.interval.ms - 5s - - Determines the sleep interval in the window. The GcTimeMonitor wakes up in - the sleep interval periodically to compute the gc time proportion. The - shorter the interval the preciser the GcTimePercentage. The sleep interval - must be shorter than the window size. - - diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java index 241f3af1b8e..237cacc7c97 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java @@ -22,7 +22,6 @@ import com.google.common.collect.LinkedListMultimap; import com.google.common.collect.Lists; import org.apache.hadoop.hdfs.protocol.SystemErasureCodingPolicies; -import org.apache.hadoop.hdfs.server.namenode.NameNode; import org.slf4j.LoggerFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.CreateFlag; @@ -343,8 +342,6 @@ private void doTestAllNodesHoldingReplicasDecommissioned(int testIndex) throws E @Test public void testOneOfTwoRacksDecommissioned() throws Exception { addNodes(nodes); - NameNode.initMetrics(new Configuration(), - HdfsServerConstants.NamenodeRole.NAMENODE); for (int i = 0; i < NUM_TEST_ITERS; i++) { doTestOneOfTwoRacksDecommissioned(i); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlocksWithNotEnoughRacks.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlocksWithNotEnoughRacks.java index 7dfb9514f9e..c0cf7ea3963 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlocksWithNotEnoughRacks.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlocksWithNotEnoughRacks.java @@ -570,8 +570,7 @@ public void testMultipleReplicasScheduledForUpgradeDomain() throws Exception { BlockInfo storedBlock = bm.getStoredBlock(b.getLocalBlock()); // The block should be replicated OK - so Reconstruction Work will be null - BlockReconstructionWork work = scheduleReconstruction( - cluster.getNamesystem(), storedBlock, 2); + BlockReconstructionWork work = bm.scheduleReconstruction(storedBlock, 2); assertNull(work); // Set the upgradeDomain to "3" for the 3 nodes hosting the block. // Then alternately set the remaining 3 nodes to have an upgradeDomain @@ -587,8 +586,7 @@ public void testMultipleReplicasScheduledForUpgradeDomain() throws Exception { } } // Now reconWork is non-null and 2 extra targets are needed - work = scheduleReconstruction( - cluster.getNamesystem(), storedBlock, 2); + work = bm.scheduleReconstruction(storedBlock, 2); assertEquals(2, work.getAdditionalReplRequired()); // Add the block to the replication queue and ensure it is replicated @@ -600,16 +598,6 @@ public void testMultipleReplicasScheduledForUpgradeDomain() throws Exception { } } - static BlockReconstructionWork scheduleReconstruction( - FSNamesystem fsn, BlockInfo block, int priority) { - fsn.writeLock(); - try { - return fsn.getBlockManager().scheduleReconstruction(block, priority); - } finally { - fsn.writeUnlock(); - } - } - @Test public void testUnderReplicatedRespectsRacksAndUpgradeDomain() throws Exception { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestCorruptionWithFailover.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestCorruptionWithFailover.java deleted file mode 100644 index f5899c0f65b..00000000000 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestCorruptionWithFailover.java +++ /dev/null @@ -1,81 +0,0 @@ -/** - * 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.hdfs.server.blockmanagement; - -import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.fs.FSDataOutputStream; -import org.apache.hadoop.fs.Path; -import org.apache.hadoop.hdfs.DistributedFileSystem; -import org.apache.hadoop.hdfs.MiniDFSCluster; -import org.apache.hadoop.hdfs.MiniDFSNNTopology; -import org.apache.hadoop.hdfs.client.HdfsClientConfigKeys; -import org.apache.hadoop.test.GenericTestUtils; -import org.junit.Test; - -/** - * Tests corruption of replicas in case of failover. - */ -public class TestCorruptionWithFailover { - - @Test - public void testCorruptReplicaAfterFailover() throws Exception { - Configuration conf = new Configuration(); - // Enable data to be written, to less replicas in case of pipeline failure. - conf.setInt(HdfsClientConfigKeys.BlockWrite.ReplaceDatanodeOnFailure. - MIN_REPLICATION, 2); - try (MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf) - .nnTopology(MiniDFSNNTopology.simpleHATopology()).numDataNodes(3) - .build()) { - cluster.transitionToActive(0); - cluster.waitActive(); - DistributedFileSystem dfs = cluster.getFileSystem(0); - FSDataOutputStream out = dfs.create(new Path("/dir/file")); - // Write some data and flush. - for (int i = 0; i < 1024 * 1024; i++) { - out.write(i); - } - out.hsync(); - // Stop one datanode, so as to trigger update pipeline. - MiniDFSCluster.DataNodeProperties dn = cluster.stopDataNode(0); - // Write some more data and close the file. - for (int i = 0; i < 1024 * 1024; i++) { - out.write(i); - } - out.close(); - BlockManager bm0 = cluster.getNamesystem(0).getBlockManager(); - BlockManager bm1 = cluster.getNamesystem(1).getBlockManager(); - // Mark datanodes as stale, as are marked if a namenode went through a - // failover, to prevent replica deletion. - bm0.getDatanodeManager().markAllDatanodesStale(); - bm1.getDatanodeManager().markAllDatanodesStale(); - // Restart the datanode - cluster.restartDataNode(dn); - // The replica from the datanode will be having lesser genstamp, so - // would be marked as CORRUPT. - GenericTestUtils.waitFor(() -> bm0.getCorruptBlocks() == 1, 100, 10000); - - // Perform failover to other namenode - cluster.transitionToStandby(0); - cluster.transitionToActive(1); - cluster.waitActive(1); - // The corrupt count should be same as first namenode. - GenericTestUtils.waitFor(() -> bm1.getCorruptBlocks() == 1, 100, 10000); - } - } -} - diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java index 1eab42a270e..d9cd4cedf05 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java @@ -31,7 +31,6 @@ import static org.apache.hadoop.fs.CommonConfigurationKeys.HA_HM_RPC_TIMEOUT_DEFAULT; import static org.apache.hadoop.fs.CommonConfigurationKeys.HA_HM_RPC_TIMEOUT_KEY; -import static org.apache.hadoop.metrics2.source.JvmMetricsInfo.GcTimePercentage; import static org.apache.hadoop.test.MetricsAsserts.assertCounter; import static org.apache.hadoop.test.MetricsAsserts.assertCounterGt; import static org.apache.hadoop.test.MetricsAsserts.assertGauge; @@ -104,7 +103,6 @@ new Path("/testNameNodeMetrics"); private static final String NN_METRICS = "NameNodeActivity"; private static final String NS_METRICS = "FSNamesystem"; - private static final String JVM_METRICS = "JvmMetrics"; private static final int BLOCK_SIZE = 1024 * 1024; private static final ErasureCodingPolicy EC_POLICY = SystemErasureCodingPolicies.getByID( @@ -225,15 +223,6 @@ public void testCapacityMetrics() throws Exception { capacityTotal); } - /** - * Test the GcTimePercentage could be got successfully. - */ - @Test - public void testGcTimePercentageMetrics() throws Exception { - MetricsRecordBuilder rb = getMetrics(JVM_METRICS); - MetricsAsserts.getIntGauge(GcTimePercentage.name(), rb); - } - /** Test metrics indicating the number of stale DataNodes */ @Test public void testStaleNodes() throws Exception { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/startupprogress/TestStartupProgress.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/startupprogress/TestStartupProgress.java index 8c0aa0ca2fb..c01844d63a7 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/startupprogress/TestStartupProgress.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/startupprogress/TestStartupProgress.java @@ -33,7 +33,6 @@ import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; -import org.apache.hadoop.hdfs.server.namenode.startupprogress.StartupProgress.Counter; import org.junit.Before; import org.junit.Test; @@ -458,15 +457,5 @@ public void testTotal() { assertEquals(800L, view.getTotal(LOADING_FSIMAGE, loadingFsImageDelegationKeys)); assertEquals(10000L, view.getTotal(LOADING_EDITS, loadingEditsFile)); - - // Try adding another step to the completed phase - // Check the step is not added and the total is not updated - Step step2 = new Step("file_2", 7000L); - startupProgress.setTotal(LOADING_EDITS, step2, 2000L); - view = startupProgress.createView(); - assertEquals(view.getTotal(LOADING_EDITS, step2), 0); - Counter counter = startupProgress.getCounter(Phase.LOADING_EDITS, step2); - counter.increment(); - assertEquals(view.getCount(LOADING_EDITS, step2), 0); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHDFS.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHDFS.java index 2b9b51fb5f4..6493b1e334f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHDFS.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHDFS.java @@ -34,7 +34,6 @@ import static org.junit.Assert.fail; import java.io.EOFException; -import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -49,7 +48,6 @@ import java.security.PrivilegedExceptionAction; import java.util.Arrays; import java.util.Collection; -import java.util.EnumSet; import java.util.Iterator; import java.util.Map; import java.util.Random; @@ -64,13 +62,11 @@ import org.apache.hadoop.fs.BlockLocation; import org.apache.hadoop.fs.BlockStoragePolicySpi; import org.apache.hadoop.fs.CommonConfigurationKeys; -import org.apache.hadoop.fs.CommonConfigurationKeysPublic; import org.apache.hadoop.fs.ContentSummary; import org.apache.hadoop.fs.FSDataInputStream; import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; -import org.apache.hadoop.fs.FileSystemTestHelper; import org.apache.hadoop.fs.FsServerDefaults; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.RemoteIterator; @@ -89,8 +85,6 @@ import org.apache.hadoop.hdfs.MiniDFSCluster; import org.apache.hadoop.hdfs.TestDFSClientRetries; import org.apache.hadoop.hdfs.TestFileCreation; -import org.apache.hadoop.hdfs.client.CreateEncryptionZoneFlag; -import org.apache.hadoop.hdfs.client.HdfsAdmin; import org.apache.hadoop.hdfs.client.HdfsClientConfigKeys; import org.apache.hadoop.hdfs.protocol.DSQuotaExceededException; import org.apache.hadoop.hdfs.protocol.ErasureCodingPolicy; @@ -1541,52 +1535,6 @@ public void testGetTrashRoot() throws Exception { assertEquals(expectedPath.toUri().getPath(), trashPath.toUri().getPath()); } - @Test - public void testGetEZTrashRoot() throws Exception { - final Configuration conf = WebHdfsTestUtil.createConf(); - FileSystemTestHelper fsHelper = new FileSystemTestHelper(); - File testRootDir = new File(fsHelper.getTestRootDir()).getAbsoluteFile(); - conf.set(CommonConfigurationKeysPublic.HADOOP_SECURITY_KEY_PROVIDER_PATH, - "jceks://file" + new Path(testRootDir.toString(), "test.jks").toUri()); - cluster = new MiniDFSCluster.Builder(conf).numDataNodes(0).build(); - cluster.waitActive(); - DistributedFileSystem dfs = cluster.getFileSystem(); - final WebHdfsFileSystem webhdfs = WebHdfsTestUtil.getWebHdfsFileSystem( - conf, WebHdfsConstants.WEBHDFS_SCHEME); - HdfsAdmin dfsAdmin = new HdfsAdmin(cluster.getURI(), conf); - dfs.getClient().setKeyProvider( - cluster.getNameNode().getNamesystem().getProvider()); - final String testkey = "test_key"; - DFSTestUtil.createKey(testkey, cluster, conf); - - final Path zone1 = new Path("/zone1"); - dfs.mkdirs(zone1, new FsPermission(700)); - dfsAdmin.createEncryptionZone(zone1, testkey, - EnumSet.of(CreateEncryptionZoneFlag.PROVISION_TRASH)); - - final Path insideEZ = new Path(zone1, "insideEZ"); - dfs.mkdirs(insideEZ, new FsPermission(700)); - assertEquals( - dfs.getTrashRoot(insideEZ).toUri().getPath(), - webhdfs.getTrashRoot(insideEZ).toUri().getPath()); - - final Path outsideEZ = new Path("/outsideEZ"); - dfs.mkdirs(outsideEZ, new FsPermission(755)); - assertEquals( - dfs.getTrashRoot(outsideEZ).toUri().getPath(), - webhdfs.getTrashRoot(outsideEZ).toUri().getPath()); - - final Path root = new Path("/"); - assertEquals( - dfs.getTrashRoot(root).toUri().getPath(), - webhdfs.getTrashRoot(root).toUri().getPath()); - assertEquals( - webhdfs.getTrashRoot(root).toUri().getPath(), - webhdfs.getTrashRoot(zone1).toUri().getPath()); - assertEquals( - webhdfs.getTrashRoot(outsideEZ).toUri().getPath(), - webhdfs.getTrashRoot(zone1).toUri().getPath()); - } @Test public void testStoragePolicy() throws Exception { diff --git a/hadoop-project/pom.xml b/hadoop-project/pom.xml index 27b1a268df0..5add6695ddb 100644 --- a/hadoop-project/pom.xml +++ b/hadoop-project/pom.xml @@ -97,7 +97,7 @@ 3.5.6 4.2.0 - 3.0.5 + 3.0.0 3.1.0-RC1 2.1.7 @@ -1329,7 +1329,7 @@ com.google.code.findbugs jsr305 - 3.0.2 + ${findbugs.version} javax.xml.bind diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java index 0ca4aa01a7e..561ab4a84a7 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java @@ -481,20 +481,6 @@ private Constants() { "fs.s3a.metadatastore.authoritative"; public static final boolean DEFAULT_METADATASTORE_AUTHORITATIVE = false; - /** - * Bucket validation parameter which can be set by client. This will be - * used in {@code S3AFileSystem.initialize(URI, Configuration)}. - * Value: {@value} - */ - public static final String S3A_BUCKET_PROBE = "fs.s3a.bucket.probe"; - - /** - * Default value of bucket validation parameter. An existence of bucket - * will be validated using {@code S3AFileSystem.verifyBucketExistsV2()}. - * Value: {@value} - */ - public static final int S3A_BUCKET_PROBE_DEFAULT = 2; - /** * How long a directory listing in the MS is considered as authoritative. */ diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java index 1e5175c1545..ce7729fa396 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java @@ -173,7 +173,6 @@ import static org.apache.hadoop.fs.s3a.auth.delegation.S3ADelegationTokens.hasDelegationTokenBinding; import static org.apache.hadoop.fs.s3a.impl.CallableSupplier.submit; import static org.apache.hadoop.fs.s3a.impl.CallableSupplier.waitForCompletionIgnoringExceptions; -import static org.apache.hadoop.fs.s3a.impl.ErrorTranslation.isUnknownBucket; import static org.apache.hadoop.fs.s3a.impl.InternalConstants.SC_404; import static org.apache.hadoop.fs.s3a.impl.NetworkBinding.fixBucketRegion; import static org.apache.hadoop.io.IOUtils.cleanupWithLogger; @@ -393,7 +392,9 @@ public void initialize(URI name, Configuration originalConf) initCannedAcls(conf); // This initiates a probe against S3 for the bucket existing. - doBucketProbing(); + // It is where all network and authentication configuration issues + // surface, and is potentially slow. + verifyBucketExists(); inputPolicy = S3AInputPolicy.getPolicy( conf.getTrimmed(INPUT_FADVISE, INPUT_FADV_NORMAL)); @@ -462,41 +463,6 @@ public void initialize(URI name, Configuration originalConf) } - /** - * Test bucket existence in S3. - * When the value of {@link Constants#S3A_BUCKET_PROBE} is set to 0, - * bucket existence check is not done to improve performance of - * S3AFileSystem initialization. When set to 1 or 2, bucket existence check - * will be performed which is potentially slow. - * If 3 or higher: warn and use the v2 check. - * @throws UnknownStoreException the bucket is absent - * @throws IOException any other problem talking to S3 - */ - @Retries.RetryTranslated - private void doBucketProbing() throws IOException { - int bucketProbe = getConf() - .getInt(S3A_BUCKET_PROBE, S3A_BUCKET_PROBE_DEFAULT); - Preconditions.checkArgument(bucketProbe >= 0, - "Value of " + S3A_BUCKET_PROBE + " should be >= 0"); - switch (bucketProbe) { - case 0: - LOG.debug("skipping check for bucket existence"); - break; - case 1: - verifyBucketExists(); - break; - case 2: - verifyBucketExistsV2(); - break; - default: - // we have no idea what this is, assume it is from a later release. - LOG.warn("Unknown bucket probe option {}: {}; falling back to check #2", - S3A_BUCKET_PROBE, bucketProbe); - verifyBucketExistsV2(); - break; - } - } - /** * Initialize the thread pool. * This must be re-invoked after replacing the S3Client during test @@ -544,31 +510,15 @@ protected static S3AStorageStatistics createStorageStatistics() { * Verify that the bucket exists. This does not check permissions, * not even read access. * Retry policy: retrying, translated. - * @throws UnknownStoreException the bucket is absent + * @throws FileNotFoundException the bucket is absent * @throws IOException any other problem talking to S3 */ @Retries.RetryTranslated protected void verifyBucketExists() - throws UnknownStoreException, IOException { + throws FileNotFoundException, IOException { if (!invoker.retry("doesBucketExist", bucket, true, () -> s3.doesBucketExist(bucket))) { - throw new UnknownStoreException("Bucket " + bucket + " does not exist"); - } - } - - /** - * Verify that the bucket exists. This will correctly throw an exception - * when credentials are invalid. - * Retry policy: retrying, translated. - * @throws UnknownStoreException the bucket is absent - * @throws IOException any other problem talking to S3 - */ - @Retries.RetryTranslated - protected void verifyBucketExistsV2() - throws UnknownStoreException, IOException { - if (!invoker.retry("doesBucketExistV2", bucket, true, - () -> s3.doesBucketExistV2(bucket))) { - throw new UnknownStoreException("Bucket " + bucket + " does not exist"); + throw new FileNotFoundException("Bucket " + bucket + " does not exist"); } } @@ -2941,7 +2891,7 @@ S3AFileStatus s3GetFileStatus(final Path path, } catch (AmazonServiceException e) { // if the response is a 404 error, it just means that there is // no file at that path...the remaining checks will be needed. - if (e.getStatusCode() != SC_404 || isUnknownBucket(e)) { + if (e.getStatusCode() != SC_404) { throw translateException("getFileStatus", path, e); } } catch (AmazonClientException e) { @@ -2973,7 +2923,7 @@ S3AFileStatus s3GetFileStatus(final Path path, meta.getVersionId()); } } catch (AmazonServiceException e) { - if (e.getStatusCode() != SC_404 || isUnknownBucket(e)) { + if (e.getStatusCode() != SC_404) { throw translateException("getFileStatus", newKey, e); } } catch (AmazonClientException e) { @@ -3012,7 +2962,7 @@ S3AFileStatus s3GetFileStatus(final Path path, return new S3AFileStatus(Tristate.TRUE, path, username); } } catch (AmazonServiceException e) { - if (e.getStatusCode() != SC_404 || isUnknownBucket(e)) { + if (e.getStatusCode() != SC_404) { throw translateException("getFileStatus", path, e); } } catch (AmazonClientException e) { diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ARetryPolicy.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ARetryPolicy.java index d2954b3a920..09e9c993b06 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ARetryPolicy.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ARetryPolicy.java @@ -188,7 +188,6 @@ protected RetryPolicy createThrottleRetryPolicy(final Configuration conf) { policyMap.put(AccessDeniedException.class, fail); policyMap.put(NoAuthWithAWSException.class, fail); policyMap.put(FileNotFoundException.class, fail); - policyMap.put(UnknownStoreException.class, fail); policyMap.put(InvalidRequestException.class, fail); // metadata stores should do retries internally when it makes sense diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java index 3775848fc8d..e2a488e8fed 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java @@ -86,7 +86,6 @@ import static org.apache.commons.lang3.StringUtils.isEmpty; import static org.apache.hadoop.fs.s3a.Constants.*; -import static org.apache.hadoop.fs.s3a.impl.ErrorTranslation.isUnknownBucket; import static org.apache.hadoop.fs.s3a.impl.MultiObjectDeleteSupport.translateDeleteException; import static org.apache.hadoop.io.IOUtils.cleanupWithLogger; @@ -250,18 +249,6 @@ public static IOException translateException(@Nullable String operation, // the object isn't there case 404: - if (isUnknownBucket(ase)) { - // this is a missing bucket - ioe = new UnknownStoreException(path, ase); - } else { - // a normal unknown object - ioe = new FileNotFoundException(message); - ioe.initCause(ase); - } - break; - - // this also surfaces sometimes and is considered to - // be ~ a not found exception. case 410: ioe = new FileNotFoundException(message); ioe.initCause(ase); diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/UnknownStoreException.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/UnknownStoreException.java deleted file mode 100644 index 0129005e067..00000000000 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/UnknownStoreException.java +++ /dev/null @@ -1,57 +0,0 @@ -/* - * 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; - -import java.io.IOException; - -import org.apache.hadoop.classification.InterfaceAudience; -import org.apache.hadoop.classification.InterfaceStability; - -/** - * The bucket or other AWS resource is unknown. - * - * Why not a subclass of FileNotFoundException? - * There's too much code which caches an FNFE and infers that the file isn't - * there - a missing bucket is far more significant and generally should - * not be ignored. - */ -@InterfaceAudience.Public -@InterfaceStability.Evolving -public class UnknownStoreException extends IOException { - - /** - * Constructor. - * @param message message - */ - public UnknownStoreException(final String message) { - this(message, null); - } - - /** - * Constructor. - * @param message message - * @param cause cause (may be null) - */ - public UnknownStoreException(final String message, Throwable cause) { - super(message); - if (cause != null) { - initCause(cause); - } - } -} diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/ErrorTranslation.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/ErrorTranslation.java deleted file mode 100644 index d1baf3c8987..00000000000 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/ErrorTranslation.java +++ /dev/null @@ -1,73 +0,0 @@ -/* - * 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 com.amazonaws.AmazonServiceException; - -import static org.apache.hadoop.fs.s3a.impl.InternalConstants.SC_404; - -/** - * Translate from AWS SDK-wrapped exceptions into IOExceptions with - * as much information as possible. - * The core of the translation logic is in S3AUtils, in - * {@code translateException} and nearby; that has grown to be - * a large a complex piece of logic, as it ties in with retry/recovery - * policies, throttling, etc. - * - * This class is where future expansion of that code should go so that we have - * an isolated place for all the changes.. - * The existing code las been left in S3AUtils it is to avoid cherry-picking - * problems on backports. - */ -public class ErrorTranslation { - - /** - * Private constructor for utility class. - */ - private ErrorTranslation() { - } - - /** - * Does this exception indicate that the AWS Bucket was unknown. - * @param e exception. - * @return true if the status code and error code mean that the - * remote bucket is unknown. - */ - public static boolean isUnknownBucket(AmazonServiceException e) { - return e.getStatusCode() == SC_404 - && AwsErrorCodes.E_NO_SUCH_BUCKET.equals(e.getErrorCode()); - } - - /** - * AWS error codes explicitly recognized and processes specially; - * kept in their own class for isolation. - */ - public static final class AwsErrorCodes { - - /** - * The AWS S3 error code used to recognize when a 404 means the bucket is - * unknown. - */ - public static final String E_NO_SUCH_BUCKET = "NoSuchBucket"; - - /** private constructor. */ - private AwsErrorCodes() { - } - } -} diff --git a/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/index.md b/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/index.md index aec778e96fb..9697e7ac40f 100644 --- a/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/index.md +++ b/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/index.md @@ -1000,26 +1000,6 @@ options are covered in [Testing](./testing.md). converged to Integer.MAX_VALUE milliseconds - - - fs.s3a.bucket.probe - 2 - - The value can be 0, 1 or 2 (default). - When set to 0, bucket existence checks won't be done - during initialization thus making it faster. - Though it should be noted that when the bucket is not available in S3, - or if fs.s3a.endpoint points to the wrong instance of a private S3 store - consecutive calls like listing, read, write etc. will fail with - an UnknownStoreException. - When set to 1, the bucket existence check will be done using the - V1 API of the S3 protocol which doesn't verify the client's permissions - to list or read data in the bucket. - When set to 2, the bucket existence check will be done using the - V2 API of the S3 protocol which does verify that the - client has permission to read the bucket. - - ``` ## Retry and Recovery diff --git a/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/performance.md b/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/performance.md index 6ca60608106..5543263471e 100644 --- a/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/performance.md +++ b/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/performance.md @@ -608,19 +608,3 @@ with HADOOP-15669. Other options may be added to `fs.s3a.ssl.channel.mode` in the future as further SSL optimizations are made. - -## Tuning FileSystem Initialization. - -When an S3A Filesystem instance is created and initialized, the client -checks if the bucket provided is valid. This can be slow. -You can ignore bucket validation by configuring `fs.s3a.bucket.probe` as follows: - -```xml - - fs.s3a.bucket.probe - 0 - -``` - -Note: if the bucket does not exist, this issue will surface when operations are performed -on the filesystem; you will see `UnknownStoreException` stack traces. diff --git a/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/troubleshooting_s3a.md b/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/troubleshooting_s3a.md index 47bc81e0ec4..5408c44aea4 100644 --- a/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/troubleshooting_s3a.md +++ b/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/troubleshooting_s3a.md @@ -1203,44 +1203,29 @@ a new one than read to the end of a large file. Note: the threshold when data is read rather than the stream aborted can be tuned by `fs.s3a.readahead.range`; seek policy in `fs.s3a.experimental.input.fadvise`. -### `UnknownStoreException` Bucket does not exist. +### `FileNotFoundException` Bucket does not exist. The bucket does not exist. ``` -org.apache.hadoop.fs.s3a.UnknownStoreException: - Bucket random-bucket-33013fb8-f7f7-4edb-9c26-16a6ed019184 does not exist - at org.apache.hadoop.fs.s3a.S3AFileSystem.verifyBucketExists(S3AFileSystem.java:537) - at org.apache.hadoop.fs.s3a.S3AFileSystem.doBucketProbing(S3AFileSystem.java:471) - at org.apache.hadoop.fs.s3a.S3AFileSystem.initialize(S3AFileSystem.java:387) - at org.apache.hadoop.fs.FileSystem.createFileSystem(FileSystem.java:3422) - at org.apache.hadoop.fs.FileSystem.get(FileSystem.java:502) +java.io.FileNotFoundException: Bucket stevel45r56666 does not exist + at org.apache.hadoop.fs.s3a.S3AFileSystem.verifyBucketExists(S3AFileSystem.java:361) + at org.apache.hadoop.fs.s3a.S3AFileSystem.initialize(S3AFileSystem.java:293) + at org.apache.hadoop.fs.FileSystem.createFileSystem(FileSystem.java:3288) + at org.apache.hadoop.fs.FileSystem.access$200(FileSystem.java:123) + at org.apache.hadoop.fs.FileSystem$Cache.getInternal(FileSystem.java:3337) + at org.apache.hadoop.fs.FileSystem$Cache.getUnique(FileSystem.java:3311) + at org.apache.hadoop.fs.FileSystem.newInstance(FileSystem.java:529) + at org.apache.hadoop.fs.s3a.s3guard.S3GuardTool$BucketInfo.run(S3GuardTool.java:997) + at org.apache.hadoop.fs.s3a.s3guard.S3GuardTool.run(S3GuardTool.java:309) + at org.apache.hadoop.util.ToolRunner.run(ToolRunner.java:76) + at org.apache.hadoop.fs.s3a.s3guard.S3GuardTool.run(S3GuardTool.java:1218) + at org.apache.hadoop.fs.s3a.s3guard.S3GuardTool.main(S3GuardTool.java:1227) ``` -Check the URI is correct, and that the bucket actually exists. -If using a third-party store, verify that you've configured +Check the URI. If using a third-party store, verify that you've configured the client to talk to the specific server in `fs.s3a.endpoint`. -Forgetting to update this value and asking the AWS S3 endpoint -for a bucket is not an unusual occurrence. - -This can surface during filesystem API calls if the bucket is deleted while you are using it, - -or the startup check for bucket existence has been disabled by setting `fs.s3a.bucket.probe` to 0. - -``` -org.apache.hadoop.fs.s3a.UnknownStoreException: s3a://random-bucket-7d9217b0-b426-4344-82ea-25d6cbb316f1/ - - at org.apache.hadoop.fs.s3a.S3AUtils.translateException(S3AUtils.java:254) - at org.apache.hadoop.fs.s3a.S3AUtils.translateException(S3AUtils.java:167) - at org.apache.hadoop.fs.s3a.S3AFileSystem.innerListFiles(S3AFileSystem.java:4149) - at org.apache.hadoop.fs.s3a.S3AFileSystem.listFiles(S3AFileSystem.java:3983) -Caused by: com.amazonaws.services.s3.model.AmazonS3Exception: -The specified bucket does not exist - (Service: Amazon S3; Status Code: 404; Error Code: NoSuchBucket - at com.amazonaws.http.AmazonHttpClient$RequestExecutor.handleErrorResponse(AmazonHttpClient.java:1712) - at com.amazonaws.http.AmazonHttpClient$RequestExecutor.executeOneRequest(AmazonHttpClient.java:1367) -``` - ## Other Issues diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/AbstractS3AMockTest.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/AbstractS3AMockTest.java index 99bab73e71c..886795a9d90 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/AbstractS3AMockTest.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/AbstractS3AMockTest.java @@ -75,7 +75,6 @@ public Configuration createConfiguration() { conf.setBoolean(CommitConstants.MAGIC_COMMITTER_ENABLED, true); // use minimum multipart size for faster triggering conf.setLong(Constants.MULTIPART_SIZE, MULTIPART_MIN_SIZE); - conf.setInt(Constants.S3A_BUCKET_PROBE, 1); return conf; } diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ABucketExistence.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ABucketExistence.java deleted file mode 100644 index 6be9003e4ec..00000000000 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ABucketExistence.java +++ /dev/null @@ -1,170 +0,0 @@ -/* - * 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; - -import java.net.URI; -import java.util.UUID; -import java.util.concurrent.Callable; - -import org.junit.Test; - -import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.fs.FileSystem; -import org.apache.hadoop.fs.Path; -import org.apache.hadoop.io.IOUtils; -import org.apache.hadoop.test.LambdaTestUtils; - -import static org.apache.hadoop.fs.contract.ContractTestUtils.dataset; -import static org.apache.hadoop.fs.contract.ContractTestUtils.writeDataset; -import static org.apache.hadoop.fs.s3a.Constants.FS_S3A; -import static org.apache.hadoop.fs.s3a.Constants.S3A_BUCKET_PROBE; -import static org.apache.hadoop.fs.s3a.Constants.S3GUARD_METASTORE_NULL; -import static org.apache.hadoop.fs.s3a.Constants.S3_METADATA_STORE_IMPL; -import static org.apache.hadoop.test.LambdaTestUtils.intercept; - -/** - * Class to test bucket existence APIs. - */ -public class ITestS3ABucketExistence extends AbstractS3ATestBase { - - private FileSystem fs; - - private final String randomBucket = - "random-bucket-" + UUID.randomUUID().toString(); - - private final URI uri = URI.create(FS_S3A + "://" + randomBucket + "/"); - - @SuppressWarnings("deprecation") - @Test - public void testNoBucketProbing() throws Exception { - describe("Disable init-time probes and expect FS operations to fail"); - Configuration conf = createConfigurationWithProbe(0); - // metastores can bypass S3 checks, so disable S3Guard, always - conf.set(S3_METADATA_STORE_IMPL, S3GUARD_METASTORE_NULL); - - fs = FileSystem.get(uri, conf); - - Path root = new Path(uri); - - expectUnknownStore( - () -> fs.getFileStatus(root)); - - expectUnknownStore( - () -> fs.listStatus(root)); - - Path src = new Path(root, "testfile"); - Path dest = new Path(root, "dst"); - expectUnknownStore( - () -> fs.getFileStatus(src)); - - // the exception must not be caught and marked down to an FNFE - expectUnknownStore(() -> fs.exists(src)); - expectUnknownStore(() -> fs.isFile(src)); - expectUnknownStore(() -> fs.isDirectory(src)); - expectUnknownStore(() -> fs.mkdirs(src)); - expectUnknownStore(() -> fs.delete(src)); - expectUnknownStore(() -> fs.rename(src, dest)); - - byte[] data = dataset(1024, 'a', 'z'); - expectUnknownStore( - () -> writeDataset(fs, src, data, data.length, 1024 * 1024, true)); - } - - /** - * Expect an operation to raise an UnknownStoreException. - * @param eval closure - * @param return type of closure - * @throws Exception anything else raised. - */ - public static void expectUnknownStore( - Callable eval) - throws Exception { - intercept(UnknownStoreException.class, eval); - } - - /** - * Expect an operation to raise an UnknownStoreException. - * @param eval closure - * @throws Exception anything else raised. - */ - public static void expectUnknownStore( - LambdaTestUtils.VoidCallable eval) - throws Exception { - intercept(UnknownStoreException.class, eval); - } - - /** - * Create a new configuration with the given bucket probe; - * we also disable FS caching. - * @param probe value to use as the bucket probe. - * @return a configuration. - */ - private Configuration createConfigurationWithProbe(final int probe) { - Configuration conf = new Configuration(getFileSystem().getConf()); - S3ATestUtils.disableFilesystemCaching(conf); - conf.setInt(S3A_BUCKET_PROBE, probe); - return conf; - } - - @Test - public void testBucketProbingV1() throws Exception { - describe("Test the V1 bucket probe"); - Configuration configuration = createConfigurationWithProbe(1); - expectUnknownStore( - () -> FileSystem.get(uri, configuration)); - } - - @Test - public void testBucketProbingV2() throws Exception { - describe("Test the V2 bucket probe"); - Configuration configuration = createConfigurationWithProbe(2); - expectUnknownStore( - () -> FileSystem.get(uri, configuration)); - /* - * Bucket probing should also be done when value of - * S3A_BUCKET_PROBE is greater than 2. - */ - configuration.setInt(S3A_BUCKET_PROBE, 3); - expectUnknownStore( - () -> FileSystem.get(uri, configuration)); - } - - @Test - public void testBucketProbingParameterValidation() throws Exception { - describe("Test bucket probe parameter %s validation", S3A_BUCKET_PROBE); - Configuration configuration = createConfigurationWithProbe(-1); - intercept(IllegalArgumentException.class, - "Value of " + S3A_BUCKET_PROBE + " should be >= 0", - "Should throw IllegalArgumentException", - () -> FileSystem.get(uri, configuration)); - } - - @Override - protected Configuration getConfiguration() { - Configuration configuration = super.getConfiguration(); - S3ATestUtils.disableFilesystemCaching(configuration); - return configuration; - } - - @Override - public void teardown() throws Exception { - IOUtils.cleanupWithLogger(getLogger(), fs); - super.teardown(); - } -} diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardOutOfBandOperations.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardOutOfBandOperations.java index 70c62bf49ca..3cdb4e6eeee 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardOutOfBandOperations.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3GuardOutOfBandOperations.java @@ -51,8 +51,6 @@ import org.apache.hadoop.test.LambdaTestUtils; import static org.apache.hadoop.fs.contract.ContractTestUtils.readBytesToString; -import static org.apache.hadoop.fs.contract.ContractTestUtils.readDataset; -import static org.apache.hadoop.fs.contract.ContractTestUtils.toChar; import static org.apache.hadoop.fs.contract.ContractTestUtils.touch; import static org.apache.hadoop.fs.contract.ContractTestUtils.writeTextFile; import static org.apache.hadoop.fs.s3a.Constants.AUTHORITATIVE_PATH; @@ -307,6 +305,11 @@ public void testListingLongerLengthOverwrite() throws Exception { overwriteFileInListing("THE TEXT", "THE LONGER TEXT"); } + @Test + public void testListingDelete() throws Exception { + deleteFileInListing(); + } + /** * Tests that tombstone expiry is implemented. If a file is created raw * while the tombstone exist in ms for with the same name then S3Guard will @@ -657,7 +660,8 @@ private void outOfBandDeletes( LOG.info("Authoritative: {} status path: {}", allowAuthoritative, status.getPath()); final boolean versionedChangeDetection = - isVersionedChangeDetection(); + getFileSystem().getChangeDetectionPolicy().getSource() + == Source.VersionId; if (!versionedChangeDetection) { expectExceptionWhenReading(testFilePath, text); expectExceptionWhenReadingOpenFileAPI(testFilePath, text, null); @@ -935,8 +939,8 @@ private void assertListingAuthority(final boolean expectAuthoritative, /** * Delete a file and use listStatus to build up the S3Guard cache. */ - @Test - public void testListingDelete() throws Exception { + private void deleteFileInListing() + throws Exception { boolean allowAuthoritative = authoritative; LOG.info("Authoritative mode enabled: {}", allowAuthoritative); @@ -965,44 +969,16 @@ public void testListingDelete() throws Exception { deleteFile(rawFS, testFilePath); // File status will be still readable from s3guard - S3AFileStatus status = (S3AFileStatus) - guardedFs.getFileStatus(testFilePath); + FileStatus status = guardedFs.getFileStatus(testFilePath); LOG.info("authoritative: {} status: {}", allowAuthoritative, status); - if (isVersionedChangeDetection() && status.getVersionId() != null) { - // when the status entry has a version ID, then that may be used - // when opening the file on what is clearly a versioned store. - int length = text.length(); - byte[] bytes = readOpenFileAPI(guardedFs, testFilePath, length, null); - Assertions.assertThat(toChar(bytes)) - .describedAs("openFile(%s)", testFilePath) - .isEqualTo(text); - // reading the rawFS with status will also work. - bytes = readOpenFileAPI(rawFS, testFilePath, length, status); - Assertions.assertThat(toChar(bytes)) - .describedAs("openFile(%s)", testFilePath) - .isEqualTo(text); - bytes = readDataset(guardedFs, testFilePath, length); - Assertions.assertThat(toChar(bytes)) - .describedAs("open(%s)", testFilePath) - .isEqualTo(text); - expectExceptionWhenReadingOpenFileAPI(rawFS, testFilePath, text, - null); - } else { - // unversioned sequence - expectExceptionWhenReading(testFilePath, text); - expectExceptionWhenReadingOpenFileAPI(testFilePath, text, null); - expectExceptionWhenReadingOpenFileAPI(testFilePath, text, status); - } + expectExceptionWhenReading(testFilePath, text); + expectExceptionWhenReadingOpenFileAPI(testFilePath, text, null); + expectExceptionWhenReadingOpenFileAPI(testFilePath, text, status); } finally { guardedFs.delete(testDirPath, true); } } - private boolean isVersionedChangeDetection() { - return getFileSystem().getChangeDetectionPolicy().getSource() - == Source.VersionId; - } - /** * We expect the read to fail with an FNFE: open will be happy. * @param testFilePath path of the test file @@ -1029,26 +1005,8 @@ private void expectExceptionWhenReading(Path testFilePath, String text) private void expectExceptionWhenReadingOpenFileAPI( Path testFilePath, String text, FileStatus status) throws Exception { - expectExceptionWhenReadingOpenFileAPI(guardedFs, - testFilePath, text, status); - } - - /** - * We expect the read to fail with an FNFE: open will be happy. - * @param fs filesystem - * @param testFilePath path of the test file - * @param text the context in the file. - * @param status optional status for the withFileStatus operation. - * @throws Exception failure other than the FNFE - */ - private void expectExceptionWhenReadingOpenFileAPI( - final S3AFileSystem fs, - final Path testFilePath - , final String text, - final FileStatus status) - throws Exception { final FutureDataInputStreamBuilder builder - = fs.openFile(testFilePath); + = guardedFs.openFile(testFilePath); if (status != null) { builder.withFileStatus(status); } @@ -1060,31 +1018,6 @@ private void expectExceptionWhenReadingOpenFileAPI( } } - /** - * Open and read a file with the openFile API. - * @param fs FS to read from - * @param testFilePath path of the test file - * @param len data length to read - * @param status optional status for the withFileStatus operation. - * @throws Exception failure - * @return the data - */ - private byte[] readOpenFileAPI( - S3AFileSystem fs, - Path testFilePath, - int len, - FileStatus status) throws Exception { - FutureDataInputStreamBuilder builder = fs.openFile(testFilePath); - if (status != null) { - builder.withFileStatus(status); - } - try (FSDataInputStream in = builder.build().get()) { - byte[] bytes = new byte[len]; - in.readFully(0, bytes); - return bytes; - } - } - /** * Wait for a deleted file to no longer be visible. * @param fs filesystem diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/MockS3ClientFactory.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/MockS3ClientFactory.java index 4644cf24764..2397f6cbafa 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/MockS3ClientFactory.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/MockS3ClientFactory.java @@ -41,7 +41,6 @@ public AmazonS3 createS3Client(URI name, final String userAgentSuffix) { AmazonS3 s3 = mock(AmazonS3.class); when(s3.doesBucketExist(bucket)).thenReturn(true); - when(s3.doesBucketExistV2(bucket)).thenReturn(true); // this listing is used in startup if purging is enabled, so // return a stub value MultipartUploadListing noUploads = new MultipartUploadListing(); diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AExceptionTranslation.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AExceptionTranslation.java index 95bd7c21b85..9b865951300 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AExceptionTranslation.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AExceptionTranslation.java @@ -21,7 +21,6 @@ import static org.apache.hadoop.fs.s3a.Constants.*; import static org.apache.hadoop.fs.s3a.S3ATestUtils.*; import static org.apache.hadoop.fs.s3a.S3AUtils.*; -import static org.apache.hadoop.fs.s3a.impl.InternalConstants.SC_404; import static org.junit.Assert.*; import java.io.EOFException; @@ -40,8 +39,6 @@ import org.junit.Test; -import org.apache.hadoop.fs.s3a.impl.ErrorTranslation; - import static org.apache.hadoop.test.GenericTestUtils.assertExceptionContains; /** @@ -101,24 +98,9 @@ public void test403isNotPermittedFound() throws Exception { verifyTranslated(403, AccessDeniedException.class); } - /** - * 404 defaults to FileNotFound. - */ @Test public void test404isNotFound() throws Exception { - verifyTranslated(SC_404, FileNotFoundException.class); - } - - /** - * 404 + NoSuchBucket == Unknown bucket. - */ - @Test - public void testUnknownBucketException() throws Exception { - AmazonS3Exception ex404 = createS3Exception(SC_404); - ex404.setErrorCode(ErrorTranslation.AwsErrorCodes.E_NO_SUCH_BUCKET); - verifyTranslated( - UnknownStoreException.class, - ex404); + verifyTranslated(404, FileNotFoundException.class); } @Test diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/AbstractS3GuardToolTestBase.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/AbstractS3GuardToolTestBase.java index aa74c002d4b..13d2646317d 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/AbstractS3GuardToolTestBase.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/AbstractS3GuardToolTestBase.java @@ -21,6 +21,7 @@ import java.io.BufferedReader; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStreamReader; import java.net.URI; @@ -35,7 +36,6 @@ import java.util.concurrent.TimeUnit; import org.apache.hadoop.fs.s3a.S3AUtils; -import org.apache.hadoop.fs.s3a.UnknownStoreException; import org.apache.hadoop.util.StopWatch; import com.google.common.base.Preconditions; import org.apache.hadoop.fs.FileSystem; @@ -506,7 +506,7 @@ public void testToolsNoBucket() throws Throwable { cmdR.getName(), S3A_THIS_BUCKET_DOES_NOT_EXIST }; - intercept(UnknownStoreException.class, + intercept(FileNotFoundException.class, () -> cmdR.run(argsR)); } } diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/ITestS3GuardToolDynamoDB.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/ITestS3GuardToolDynamoDB.java index ba93927e8dc..915f1cc190c 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/ITestS3GuardToolDynamoDB.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/ITestS3GuardToolDynamoDB.java @@ -41,7 +41,6 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.s3a.Constants; import org.apache.hadoop.fs.s3a.S3AFileSystem; -import org.apache.hadoop.fs.s3a.UnknownStoreException; import org.apache.hadoop.fs.s3a.s3guard.S3GuardTool.Destroy; import org.apache.hadoop.fs.s3a.s3guard.S3GuardTool.Init; import org.apache.hadoop.util.ExitUtil; @@ -320,7 +319,7 @@ public void testCLIFsckWithParamParentOfRoot() throws Exception { @Test public void testCLIFsckFailInitializeFs() throws Exception { - intercept(UnknownStoreException.class, + intercept(FileNotFoundException.class, "does not exist", () -> run(S3GuardTool.Fsck.NAME, "-check", "s3a://this-bucket-does-not-exist-" + UUID.randomUUID())); } diff --git a/hadoop-tools/hadoop-aws/src/test/resources/core-site.xml b/hadoop-tools/hadoop-aws/src/test/resources/core-site.xml index a90edbe24fc..5fd7c25f246 100644 --- a/hadoop-tools/hadoop-aws/src/test/resources/core-site.xml +++ b/hadoop-tools/hadoop-aws/src/test/resources/core-site.xml @@ -51,12 +51,6 @@ managed by s3guard - - fs.s3a.bucket.landsat-pds.probe - 0 - Let's postpone existence checks to the first IO operation - - s3guard.null diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/AzureServiceErrorCode.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/AzureServiceErrorCode.java index 8bc31c4f92b..60e7f92d270 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/AzureServiceErrorCode.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/AzureServiceErrorCode.java @@ -46,7 +46,6 @@ EGRESS_OVER_ACCOUNT_LIMIT(null, HttpURLConnection.HTTP_UNAVAILABLE, "Egress is over the account limit."), INVALID_QUERY_PARAMETER_VALUE("InvalidQueryParameterValue", HttpURLConnection.HTTP_BAD_REQUEST, null), AUTHORIZATION_PERMISSION_MISS_MATCH("AuthorizationPermissionMismatch", HttpURLConnection.HTTP_FORBIDDEN, null), - ACCOUNT_REQUIRES_HTTPS("AccountRequiresHttps", HttpURLConnection.HTTP_BAD_REQUEST, null), UNKNOWN(null, -1, null); private final String errorCode; diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsInputStream.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsInputStream.java index 8dc3b8f0707..1f343424fbf 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsInputStream.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsInputStream.java @@ -25,22 +25,16 @@ import com.google.common.base.Preconditions; -import org.apache.hadoop.fs.CanUnbuffer; import org.apache.hadoop.fs.FSExceptionMessages; import org.apache.hadoop.fs.FSInputStream; import org.apache.hadoop.fs.FileSystem.Statistics; -import org.apache.hadoop.fs.StreamCapabilities; import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException; import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AzureBlobFileSystemException; -import static org.apache.hadoop.util.StringUtils.toLowerCase; - /** * The AbfsInputStream for AbfsClient. */ -public class AbfsInputStream extends FSInputStream implements CanUnbuffer, - StreamCapabilities { - +public class AbfsInputStream extends FSInputStream { private final AbfsClient client; private final Statistics statistics; private final String path; @@ -396,23 +390,4 @@ public synchronized void reset() throws IOException { public boolean markSupported() { return false; } - - @Override - public synchronized void unbuffer() { - buffer = null; - // Preserve the original position returned by getPos() - fCursor = fCursor - limit + bCursor; - fCursorAfterLastRead = -1; - bCursor = 0; - limit = 0; - } - - @Override - public boolean hasCapability(String capability) { - return StreamCapabilities.UNBUFFER.equals(toLowerCase(capability)); - } - - byte[] getBuffer() { - return buffer; - } } diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestClientUrlScheme.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestClientUrlScheme.java index 44665f50c11..ad889838ff1 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestClientUrlScheme.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestClientUrlScheme.java @@ -29,8 +29,6 @@ import org.junit.runners.Parameterized; import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException; -import org.apache.hadoop.fs.azurebfs.contracts.services.AzureServiceErrorCode; import org.apache.hadoop.fs.azurebfs.services.AbfsClient; import org.apache.hadoop.fs.CommonConfigurationKeysPublic; import org.apache.hadoop.fs.azurebfs.constants.FileSystemUriSchemes; @@ -83,25 +81,8 @@ public void testClientUrlScheme() throws Exception { Configuration config = getRawConfiguration(); config.set(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY, fsUrl.toString()); config.setBoolean(FS_AZURE_ALWAYS_USE_HTTPS, alwaysUseHttps); - // HTTP is enabled only when "abfs://XXX" is used and FS_AZURE_ALWAYS_USE_HTTPS - // is set as false, otherwise HTTPS should be used. - boolean expectHttpConnection = !useSecureScheme && !alwaysUseHttps; - - AbfsClient client = null; - try { - client = this.getFileSystem(config).getAbfsClient(); - } catch (AbfsRestOperationException e) { - if (AzureServiceErrorCode.ACCOUNT_REQUIRES_HTTPS.equals(e.getErrorCode()) - && expectHttpConnection) { - // if we get here, the error message was the account supports HTTPS only - // and this parameterized test is trying to create an HTTP one. - // we can implicitly infer that the scheme setup went through, - // otherwise it would not have been rejected at the far end - return; - } else { - throw e; - } - } + + AbfsClient client = this.getFileSystem(config).getAbfsClient(); Field baseUrlField = AbfsClient.class. getDeclaredField("baseUrl"); @@ -109,7 +90,9 @@ public void testClientUrlScheme() throws Exception { String url = ((URL) baseUrlField.get(client)).toString(); - if (expectHttpConnection) { + // HTTP is enabled only when "abfs://XXX" is used and FS_AZURE_ALWAYS_USE_HTTPS + // is set as false, otherwise HTTPS should be used. + if (!useSecureScheme && !alwaysUseHttps) { Assert.assertTrue(url.startsWith(FileSystemUriSchemes.HTTP_SCHEME)); } else { Assert.assertTrue(url.startsWith(FileSystemUriSchemes.HTTPS_SCHEME)); diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/contract/ITestAbfsContractUnbuffer.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/contract/ITestAbfsContractUnbuffer.java deleted file mode 100644 index 3f7fce41a0b..00000000000 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/contract/ITestAbfsContractUnbuffer.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.hadoop.fs.azurebfs.contract; - -import java.io.IOException; - -import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.fs.FSDataInputStream; -import org.apache.hadoop.fs.contract.AbstractContractUnbufferTest; - -/** - * Contract test for unbuffer operation. - */ -public class ITestAbfsContractUnbuffer extends AbstractContractUnbufferTest { - private final boolean isSecure; - private final ABFSContractTestBinding binding; - - public ITestAbfsContractUnbuffer() throws Exception { - binding = new ABFSContractTestBinding(); - this.isSecure = binding.isSecureMode(); - } - - @Override - public void setup() throws Exception { - binding.setup(); - super.setup(); - } - - @Override - protected Configuration createConfiguration() { - return binding.getRawConfiguration(); - } - - @Override - protected AbfsFileSystemContract createContract(Configuration conf) { - return new AbfsFileSystemContract(conf, isSecure); - } - - /** - * {@link org.apache.hadoop.fs.azurebfs.services.AbfsInputStream} does not - * allow calling {@link org.apache.hadoop.fs.Seekable#getPos()} on a closed - * stream, so this test needs to be overridden so that it does not call - * getPos() after the stream has been closed. - */ - @Override - public void testUnbufferOnClosedFile() throws IOException { - describe("unbuffer a file before a read"); - FSDataInputStream stream = null; - try { - stream = getFileSystem().open(getFile()); - validateFullFileContents(stream); - } finally { - if (stream != null) { - stream.close(); - } - } - if (stream != null) { - stream.unbuffer(); - } - } -} diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsUnbuffer.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsUnbuffer.java deleted file mode 100644 index 7c96a950e23..00000000000 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsUnbuffer.java +++ /dev/null @@ -1,84 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.hadoop.fs.azurebfs.services; - -import java.io.IOException; - -import org.junit.Test; - -import org.apache.hadoop.fs.FSDataInputStream; -import org.apache.hadoop.fs.Path; -import org.apache.hadoop.fs.azurebfs.AbstractAbfsIntegrationTest; -import org.apache.hadoop.fs.contract.ContractTestUtils; - -/** - * Integration test for calling - * {@link org.apache.hadoop.fs.CanUnbuffer#unbuffer} on {@link AbfsInputStream}. - * Validates that the underlying stream's buffer is null. - */ -public class ITestAbfsUnbuffer extends AbstractAbfsIntegrationTest { - - private Path dest; - - public ITestAbfsUnbuffer() throws Exception { - } - - @Override - public void setup() throws Exception { - super.setup(); - dest = path("ITestAbfsUnbuffer"); - - byte[] data = ContractTestUtils.dataset(16, 'a', 26); - ContractTestUtils.writeDataset(getFileSystem(), dest, data, data.length, - 16, true); - } - - @Test - public void testUnbuffer() throws IOException { - // Open file, read half the data, and then call unbuffer - try (FSDataInputStream inputStream = getFileSystem().open(dest)) { - assertTrue("unexpected stream type " - + inputStream.getWrappedStream().getClass().getSimpleName(), - inputStream.getWrappedStream() instanceof AbfsInputStream); - readAndAssertBytesRead(inputStream, 8); - assertFalse("AbfsInputStream buffer should not be null", - isBufferNull(inputStream)); - inputStream.unbuffer(); - - // Check the the underlying buffer is null - assertTrue("AbfsInputStream buffer should be null", - isBufferNull(inputStream)); - } - } - - private boolean isBufferNull(FSDataInputStream inputStream) { - return ((AbfsInputStream) inputStream.getWrappedStream()).getBuffer() == null; - } - - /** - * Read the specified number of bytes from the given - * {@link FSDataInputStream} and assert that - * {@link FSDataInputStream#read(byte[])} read the specified number of bytes. - */ - private static void readAndAssertBytesRead(FSDataInputStream inputStream, - int bytesToRead) throws IOException { - assertEquals("AbfsInputStream#read did not read the correct number of " - + "bytes", bytesToRead, inputStream.read(new byte[bytesToRead])); - } -} diff --git a/hadoop-tools/hadoop-azure/src/test/resources/abfs.xml b/hadoop-tools/hadoop-azure/src/test/resources/abfs.xml index 1561da2234c..d065ace8b58 100644 --- a/hadoop-tools/hadoop-azure/src/test/resources/abfs.xml +++ b/hadoop-tools/hadoop-azure/src/test/resources/abfs.xml @@ -61,9 +61,4 @@ fs.contract.supports-getfilestatus true - - - fs.contract.supports-unbuffer - true - diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/AMRMProxyMetrics.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/AMRMProxyMetrics.java new file mode 100644 index 00000000000..f4f07707bf5 --- /dev/null +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/AMRMProxyMetrics.java @@ -0,0 +1,191 @@ +/** + * 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.yarn.server.nodemanager.amrmproxy; + +import com.google.common.annotations.VisibleForTesting; +import org.apache.hadoop.metrics2.MetricsInfo; +import org.apache.hadoop.metrics2.annotation.Metric; +import org.apache.hadoop.metrics2.annotation.Metrics; +import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem; +import org.apache.hadoop.metrics2.lib.MetricsRegistry; +import org.apache.hadoop.metrics2.lib.MutableGaugeLong; +import org.apache.hadoop.metrics2.lib.MutableQuantiles; +import org.apache.hadoop.metrics2.lib.MutableRate; + + +import static org.apache.hadoop.metrics2.lib.Interns.info; + +@Metrics(about = "Metrics for AMRMProxy", context = "fedr") +public class AMRMProxyMetrics { + + private final MetricsInfo RECORD_INFO = + info("AMRMProxyMetrics", "Metrics for the AMRMProxy"); + @Metric("# of failed applications start requests") private MutableGaugeLong + failedAppStartRequests; + @Metric("# of failed register AM requests") private MutableGaugeLong + failedRegisterAMRequests; + @Metric("# of failed finish AM requests") private MutableGaugeLong + failedFinishAMRequests; + @Metric("# of failed allocate requests ") private MutableGaugeLong + failedAllocateRequests; + @Metric("# of failed application recoveries") private MutableGaugeLong + failedAppRecoveryCount; + // Aggregate metrics are shared, and don't have to be looked up per call + @Metric("Application start request latency(ms)") private MutableRate + totalSucceededAppStartRequests; + @Metric("Register application master latency(ms)") private MutableRate + totalSucceededRegisterAMRequests; + @Metric("Finish application master latency(ms)") private MutableRate + totalSucceededFinishAMRequests; + @Metric("Allocate latency(ms)") private MutableRate + totalSucceededAllocateRequests; + // Quantile latency in ms - this is needed for SLA (95%, 99%, etc) + private MutableQuantiles applicationStartLatency; + private MutableQuantiles registerAMLatency; + private MutableQuantiles finishAMLatency; + private MutableQuantiles allocateLatency; + private static volatile AMRMProxyMetrics INSTANCE = null; + private MetricsRegistry registry; + + private AMRMProxyMetrics() { + registry = new MetricsRegistry(RECORD_INFO); + registry.tag(RECORD_INFO, "AMRMProxy"); + + applicationStartLatency = registry + .newQuantiles("applicationStartLatency", "latency of app start", "ops", + "latency", 10); + registerAMLatency = registry + .newQuantiles("registerAMLatency", "latency of register AM", "ops", + "latency", 10); + finishAMLatency = registry + .newQuantiles("finishAMLatency", "latency of finish AM", "ops", + "latency", 10); + allocateLatency = registry + .newQuantiles("allocateLatency", "latency of allocate", "ops", + "latency", 10); + } + + /** + * Initialize the singleton instance. + * + * @return the singleton + */ + public static AMRMProxyMetrics getMetrics() { + synchronized (AMRMProxyMetrics.class) { + if (INSTANCE == null) { + INSTANCE = DefaultMetricsSystem.instance() + .register("AMRMProxyMetrics", "Metrics for the Yarn AMRMProxy", + new AMRMProxyMetrics()); + } + } + return INSTANCE; + } + + @VisibleForTesting public long getNumSucceededAppStartRequests() { + return totalSucceededAppStartRequests.lastStat().numSamples(); + } + + @VisibleForTesting public double getLatencySucceededAppStartRequests() { + return totalSucceededAppStartRequests.lastStat().mean(); + } + + public void succeededAppStartRequests(long duration) { + totalSucceededAppStartRequests.add(duration); + applicationStartLatency.add(duration); + } + + @VisibleForTesting public long getNumSucceededRegisterAMRequests() { + return totalSucceededRegisterAMRequests.lastStat().numSamples(); + } + + @VisibleForTesting public double getLatencySucceededRegisterAMRequests() { + return totalSucceededRegisterAMRequests.lastStat().mean(); + } + + public void succeededRegisterAMRequests(long duration) { + totalSucceededRegisterAMRequests.add(duration); + registerAMLatency.add(duration); + } + + @VisibleForTesting public long getNumSucceededFinishAMRequests() { + return totalSucceededFinishAMRequests.lastStat().numSamples(); + } + + @VisibleForTesting public double getLatencySucceededFinishAMRequests() { + return totalSucceededFinishAMRequests.lastStat().mean(); + } + + public void succeededFinishAMRequests(long duration) { + totalSucceededFinishAMRequests.add(duration); + finishAMLatency.add(duration); + } + + @VisibleForTesting public long getNumSucceededAllocateRequests() { + return totalSucceededAllocateRequests.lastStat().numSamples(); + } + + @VisibleForTesting public double getLatencySucceededAllocateRequests() { + return totalSucceededAllocateRequests.lastStat().mean(); + } + + public void succeededAllocateRequests(long duration) { + totalSucceededAllocateRequests.add(duration); + allocateLatency.add(duration); + } + + public long getFailedAppStartRequests() { + return failedAppStartRequests.value(); + } + + public void incrFailedAppStartRequests() { + failedAppStartRequests.incr(); + } + + public long getFailedRegisterAMRequests() { + return failedRegisterAMRequests.value(); + } + + public void incrFailedRegisterAMRequests() { + failedRegisterAMRequests.incr(); + } + + public long getFailedFinishAMRequests() { + return failedFinishAMRequests.value(); + } + + public void incrFailedFinishAMRequests() { + failedFinishAMRequests.incr(); + } + + public long getFailedAllocateRequests() { + return failedAllocateRequests.value(); + } + + public void incrFailedAllocateRequests() { + failedAllocateRequests.incr(); + } + + public long getFailedAppRecoveryCount() { + return failedAppRecoveryCount.value(); + } + + public void incrFailedAppRecoveryCount() { + failedAppRecoveryCount.incr(); + } +} diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/AMRMProxyService.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/AMRMProxyService.java index 52f33135084..d3c4a1d5288 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/AMRMProxyService.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/AMRMProxyService.java @@ -75,7 +75,9 @@ import org.apache.hadoop.yarn.server.security.MasterKeyData; import org.apache.hadoop.yarn.server.utils.BuilderUtils; import org.apache.hadoop.yarn.server.utils.YarnServerSecurityUtils; +import org.apache.hadoop.yarn.util.Clock; import org.apache.hadoop.yarn.util.ConverterUtils; +import org.apache.hadoop.yarn.util.MonotonicClock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -97,6 +99,7 @@ private static final String NMSS_USER_KEY = "user"; private static final String NMSS_AMRMTOKEN_KEY = "amrmtoken"; + private final Clock clock = new MonotonicClock(); private Server server; private final Context nmContext; private final AsyncDispatcher dispatcher; @@ -104,6 +107,7 @@ private AMRMProxyTokenSecretManager secretManager; private Map applPipelineMap; private RegistryOperations registry; + private AMRMProxyMetrics metrics; /** * Creates an instance of the service. @@ -122,6 +126,8 @@ public AMRMProxyService(Context nmContext, AsyncDispatcher dispatcher) { this.dispatcher.register(ApplicationEventType.class, new ApplicationEventHandler()); + + metrics = AMRMProxyMetrics.getMetrics(); } @Override @@ -272,6 +278,7 @@ public void recover() throws IOException { } catch (Throwable e) { LOG.error("Exception when recovering " + attemptId + ", removing it from NMStateStore and move on", e); + this.metrics.incrFailedAppRecoveryCount(); this.nmContext.getNMStateStore().removeAMRMProxyAppContext(attemptId); } } @@ -286,13 +293,26 @@ public void recover() throws IOException { public RegisterApplicationMasterResponse registerApplicationMaster( RegisterApplicationMasterRequest request) throws YarnException, IOException { - LOG.info("Registering application master." + " Host:" - + request.getHost() + " Port:" + request.getRpcPort() - + " Tracking Url:" + request.getTrackingUrl()); - RequestInterceptorChainWrapper pipeline = - authorizeAndGetInterceptorChain(); - return pipeline.getRootInterceptor() - .registerApplicationMaster(request); + long startTime = clock.getTime(); + try { + RequestInterceptorChainWrapper pipeline = + authorizeAndGetInterceptorChain(); + LOG.info("Registering application master." + " Host:" + request.getHost() + + " Port:" + request.getRpcPort() + " Tracking Url:" + request + .getTrackingUrl() + " for application " + pipeline + .getApplicationAttemptId()); + RegisterApplicationMasterResponse response = + pipeline.getRootInterceptor().registerApplicationMaster(request); + + long endTime = clock.getTime(); + this.metrics.succeededRegisterAMRequests(endTime - startTime); + LOG.info("RegisterAM processing finished in {} ms for application {}", + endTime - startTime, pipeline.getApplicationAttemptId()); + return response; + } catch (Throwable t) { + this.metrics.incrFailedRegisterAMRequests(); + throw t; + } } /** @@ -304,11 +324,25 @@ public RegisterApplicationMasterResponse registerApplicationMaster( public FinishApplicationMasterResponse finishApplicationMaster( FinishApplicationMasterRequest request) throws YarnException, IOException { - LOG.info("Finishing application master. Tracking Url:" - + request.getTrackingUrl()); - RequestInterceptorChainWrapper pipeline = - authorizeAndGetInterceptorChain(); - return pipeline.getRootInterceptor().finishApplicationMaster(request); + long startTime = clock.getTime(); + try { + RequestInterceptorChainWrapper pipeline = + authorizeAndGetInterceptorChain(); + LOG.info("Finishing application master for {}. Tracking Url: {}", + pipeline.getApplicationAttemptId(), request.getTrackingUrl()); + FinishApplicationMasterResponse response = + pipeline.getRootInterceptor().finishApplicationMaster(request); + + long endTime = clock.getTime(); + this.metrics.succeededFinishAMRequests(endTime - startTime); + LOG.info("FinishAM finished with isUnregistered = {} in {} ms for {}", + response.getIsUnregistered(), endTime - startTime, + pipeline.getApplicationAttemptId()); + return response; + } catch (Throwable t) { + this.metrics.incrFailedFinishAMRequests(); + throw t; + } } /** @@ -321,16 +355,26 @@ public FinishApplicationMasterResponse finishApplicationMaster( @Override public AllocateResponse allocate(AllocateRequest request) throws YarnException, IOException { - AMRMTokenIdentifier amrmTokenIdentifier = - YarnServerSecurityUtils.authorizeRequest(); - RequestInterceptorChainWrapper pipeline = - getInterceptorChain(amrmTokenIdentifier); - AllocateResponse allocateResponse = - pipeline.getRootInterceptor().allocate(request); - - updateAMRMTokens(amrmTokenIdentifier, pipeline, allocateResponse); - - return allocateResponse; + long startTime = clock.getTime(); + try { + AMRMTokenIdentifier amrmTokenIdentifier = + YarnServerSecurityUtils.authorizeRequest(); + RequestInterceptorChainWrapper pipeline = + getInterceptorChain(amrmTokenIdentifier); + AllocateResponse allocateResponse = + pipeline.getRootInterceptor().allocate(request); + + updateAMRMTokens(amrmTokenIdentifier, pipeline, allocateResponse); + + long endTime = clock.getTime(); + this.metrics.succeededAllocateRequests(endTime - startTime); + LOG.info("Allocate processing finished in {} ms for application {}", + endTime - startTime, pipeline.getApplicationAttemptId()); + return allocateResponse; + } catch (Throwable t) { + this.metrics.incrFailedAllocateRequests(); + throw t; + } } /** @@ -343,40 +387,47 @@ public AllocateResponse allocate(AllocateRequest request) */ public void processApplicationStartRequest(StartContainerRequest request) throws IOException, YarnException { - LOG.info("Callback received for initializing request " - + "processing pipeline for an AM"); - ContainerTokenIdentifier containerTokenIdentifierForKey = - BuilderUtils.newContainerTokenIdentifier(request - .getContainerToken()); - ApplicationAttemptId appAttemptId = - containerTokenIdentifierForKey.getContainerID() - .getApplicationAttemptId(); - Credentials credentials = - YarnServerSecurityUtils.parseCredentials(request - .getContainerLaunchContext()); - - Token amrmToken = - getFirstAMRMToken(credentials.getAllTokens()); - if (amrmToken == null) { - throw new YarnRuntimeException( - "AMRMToken not found in the start container request for application:" - + appAttemptId.toString()); - } - - // Substitute the existing AMRM Token with a local one. Keep the rest of the - // tokens in the credentials intact. - Token localToken = - this.secretManager.createAndGetAMRMToken(appAttemptId); - credentials.addToken(localToken.getService(), localToken); - - DataOutputBuffer dob = new DataOutputBuffer(); - credentials.writeTokenStorageToStream(dob); - request.getContainerLaunchContext().setTokens( - ByteBuffer.wrap(dob.getData(), 0, dob.getLength())); + long startTime = clock.getTime(); + try { + LOG.info("Callback received for initializing request " + + "processing pipeline for an AM"); + ContainerTokenIdentifier containerTokenIdentifierForKey = + BuilderUtils.newContainerTokenIdentifier(request.getContainerToken()); + ApplicationAttemptId appAttemptId = + containerTokenIdentifierForKey.getContainerID() + .getApplicationAttemptId(); + Credentials credentials = YarnServerSecurityUtils + .parseCredentials(request.getContainerLaunchContext()); + + Token amrmToken = + getFirstAMRMToken(credentials.getAllTokens()); + if (amrmToken == null) { + throw new YarnRuntimeException( + "AMRMToken not found in the start container request for " + + "application:" + appAttemptId.toString()); + } - initializePipeline(appAttemptId, - containerTokenIdentifierForKey.getApplicationSubmitter(), amrmToken, - localToken, null, false, credentials); + // Substitute the existing AMRM Token with a local one. Keep the rest of + // the tokens in the credentials intact. + Token localToken = + this.secretManager.createAndGetAMRMToken(appAttemptId); + credentials.addToken(localToken.getService(), localToken); + + DataOutputBuffer dob = new DataOutputBuffer(); + credentials.writeTokenStorageToStream(dob); + request.getContainerLaunchContext() + .setTokens(ByteBuffer.wrap(dob.getData(), 0, dob.getLength())); + + initializePipeline(appAttemptId, + containerTokenIdentifierForKey.getApplicationSubmitter(), amrmToken, + localToken, null, false, credentials); + + long endTime = clock.getTime(); + this.metrics.succeededAppStartRequests(endTime - startTime); + } catch (Throwable t) { + this.metrics.incrFailedAppStartRequests(); + throw t; + } } /** diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/TestAMRMProxyMetrics.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/TestAMRMProxyMetrics.java new file mode 100644 index 00000000000..f7cadd1bf32 --- /dev/null +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/amrmproxy/TestAMRMProxyMetrics.java @@ -0,0 +1,163 @@ +/** + * 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.yarn.server.nodemanager.amrmproxy; + +import org.apache.hadoop.yarn.api.protocolrecords.AllocateResponse; +import org.apache.hadoop.yarn.api.protocolrecords.FinishApplicationMasterResponse; +import org.apache.hadoop.yarn.api.protocolrecords.RegisterApplicationMasterResponse; +import org.apache.hadoop.yarn.api.records.FinalApplicationStatus; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class TestAMRMProxyMetrics extends BaseAMRMProxyTest { + public static final Logger LOG = + LoggerFactory.getLogger(TestAMRMProxyMetrics.class); + private static AMRMProxyMetrics metrics; + + @BeforeClass public static void init() { + metrics = AMRMProxyMetrics.getMetrics(); + LOG.info("Test: aggregate metrics are initialized correctly"); + + Assert.assertEquals(0, metrics.getFailedAppStartRequests()); + Assert.assertEquals(0, metrics.getFailedRegisterAMRequests()); + Assert.assertEquals(0, metrics.getFailedFinishAMRequests()); + Assert.assertEquals(0, metrics.getFailedAllocateRequests()); + + Assert.assertEquals(0, metrics.getNumSucceededAppStartRequests()); + Assert + .assertEquals(0, metrics.getNumSucceededRegisterAMRequests()); + Assert.assertEquals(0, metrics.getNumSucceededFinishAMRequests()); + Assert.assertEquals(0, metrics.getNumSucceededAllocateRequests()); + + LOG.info("Test: aggregate metrics are updated correctly"); + } + + @Test public void testAllocateRequestWithNullValues() throws Exception { + long failedAppStartRequests = metrics.getFailedAppStartRequests(); + long failedRegisterAMRequests = + metrics.getFailedRegisterAMRequests(); + long failedFinishAMRequests = metrics.getFailedFinishAMRequests(); + long failedAllocateRequests = metrics.getFailedAllocateRequests(); + + long succeededAppStartRequests = + metrics.getNumSucceededAppStartRequests(); + long succeededRegisterAMRequests = + metrics.getNumSucceededRegisterAMRequests(); + long succeededFinishAMRequests = + metrics.getNumSucceededFinishAMRequests(); + long succeededAllocateRequests = + metrics.getNumSucceededAllocateRequests(); + + int testAppId = 1; + RegisterApplicationMasterResponse registerResponse = + registerApplicationMaster(testAppId); + Assert.assertNotNull(registerResponse); + Assert + .assertEquals(Integer.toString(testAppId), registerResponse.getQueue()); + + AllocateResponse allocateResponse = allocate(testAppId); + Assert.assertNotNull(allocateResponse); + + FinishApplicationMasterResponse finshResponse = + finishApplicationMaster(testAppId, FinalApplicationStatus.SUCCEEDED); + + Assert.assertNotNull(finshResponse); + Assert.assertEquals(true, finshResponse.getIsUnregistered()); + + Assert.assertEquals(failedAppStartRequests, + metrics.getFailedAppStartRequests()); + Assert.assertEquals(failedRegisterAMRequests, + metrics.getFailedRegisterAMRequests()); + Assert.assertEquals(failedFinishAMRequests, + metrics.getFailedFinishAMRequests()); + Assert.assertEquals(failedAllocateRequests, + metrics.getFailedAllocateRequests()); + + Assert.assertEquals(succeededAppStartRequests, + metrics.getNumSucceededAppStartRequests()); + Assert.assertEquals(1 + succeededRegisterAMRequests, + metrics.getNumSucceededRegisterAMRequests()); + Assert.assertEquals(1 + succeededFinishAMRequests, + metrics.getNumSucceededFinishAMRequests()); + Assert.assertEquals(1 + succeededAllocateRequests, + metrics.getNumSucceededAllocateRequests()); + } + + @Test public void testFinishOneApplicationMasterWithFailure() + throws Exception { + long failedAppStartRequests = metrics.getFailedAppStartRequests(); + long failedRegisterAMRequests = + metrics.getFailedRegisterAMRequests(); + long failedFinishAMRequests = metrics.getFailedFinishAMRequests(); + long failedAllocateRequests = metrics.getFailedAllocateRequests(); + + long succeededAppStartRequests = + metrics.getNumSucceededAppStartRequests(); + long succeededRegisterAMRequests = + metrics.getNumSucceededRegisterAMRequests(); + long succeededFinishAMRequests = + metrics.getNumSucceededFinishAMRequests(); + long succeededAllocateRequests = + metrics.getNumSucceededAllocateRequests(); + + int testAppId = 1; + RegisterApplicationMasterResponse registerResponse = + registerApplicationMaster(testAppId); + Assert.assertNotNull(registerResponse); + Assert + .assertEquals(Integer.toString(testAppId), registerResponse.getQueue()); + + FinishApplicationMasterResponse finshResponse = + finishApplicationMaster(testAppId, FinalApplicationStatus.FAILED); + + Assert.assertNotNull(finshResponse); + + try { + // Try to finish an application master that is already finished. + finishApplicationMaster(testAppId, FinalApplicationStatus.SUCCEEDED); + Assert + .fail("The request to finish application master should have failed"); + } catch (Throwable ex) { + // This is expected. So nothing required here. + LOG.info("Finish registration failed as expected because it was not " + + "registered"); + } + + Assert.assertEquals(failedAppStartRequests, + metrics.getFailedAppStartRequests()); + Assert.assertEquals(failedRegisterAMRequests, + metrics.getFailedRegisterAMRequests()); + Assert.assertEquals(1 + failedFinishAMRequests, + metrics.getFailedFinishAMRequests()); + Assert.assertEquals(failedAllocateRequests, + metrics.getFailedAllocateRequests()); + + Assert.assertEquals(succeededAppStartRequests, + metrics.getNumSucceededAppStartRequests()); + Assert.assertEquals(1 + succeededRegisterAMRequests, + metrics.getNumSucceededRegisterAMRequests()); + Assert.assertEquals(1 + succeededFinishAMRequests, + metrics.getNumSucceededFinishAMRequests()); + Assert.assertEquals(succeededAllocateRequests, + metrics.getNumSucceededAllocateRequests()); + } +} diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/FSConfigToCSConfigConverter.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/FSConfigToCSConfigConverter.java index b0fcc665f34..efc8f0e95cb 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/FSConfigToCSConfigConverter.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/FSConfigToCSConfigConverter.java @@ -166,13 +166,11 @@ private Resource getClusterResource( private void loadConversionRules(String rulesFile) throws IOException { if (rulesFile != null) { LOG.info("Reading conversion rules file from: " + rulesFile); - ruleHandler.loadRulesFromFile(rulesFile); + this.ruleHandler.loadRulesFromFile(rulesFile); } else { LOG.info("Conversion rules file is not defined, " + "using default conversion config!"); } - - ruleHandler.initPropertyActions(); } private Configuration getInputYarnSiteConfig( diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/FSConfigToCSConfigRuleHandler.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/FSConfigToCSConfigRuleHandler.java index f4526cb7bf2..a1db393e4c6 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/FSConfigToCSConfigRuleHandler.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/FSConfigToCSConfigRuleHandler.java @@ -76,9 +76,6 @@ public static final String FAIR_AS_DRF = "fairAsDrf.action"; - public static final String MAPPED_DYNAMIC_QUEUE = - "mappedDynamicQueue.action"; - @VisibleForTesting enum RuleAction { WARNING, @@ -98,6 +95,7 @@ void loadRulesFromFile(String ruleFile) throws IOException { properties.load(is); } actions = new HashMap<>(); + initPropertyActions(); } public FSConfigToCSConfigRuleHandler(ConversionOptions conversionOptions) { @@ -115,7 +113,7 @@ public FSConfigToCSConfigRuleHandler(ConversionOptions conversionOptions) { initPropertyActions(); } - public void initPropertyActions() { + private void initPropertyActions() { setActionForProperty(MAX_CAPACITY_PERCENTAGE); setActionForProperty(MAX_CHILD_CAPACITY); setActionForProperty(USER_MAX_RUNNING_APPS); @@ -125,7 +123,6 @@ public void initPropertyActions() { setActionForProperty(RESERVATION_SYSTEM); setActionForProperty(QUEUE_AUTO_CREATE); setActionForProperty(FAIR_AS_DRF); - setActionForProperty(MAPPED_DYNAMIC_QUEUE); } public void handleMaxCapacityPercentage(String queueName) { @@ -184,8 +181,7 @@ public void handleQueueAutoCreate(String placementRule) { handle(QUEUE_AUTO_CREATE, null, format( - "Placement rules: queue auto-create is not supported (type: %s)," - + " please configure auto-create-child-queue property manually", + "Placement rules: queue auto-create is not supported (type: %s)", placementRule)); } @@ -197,21 +193,6 @@ public void handleFairAsDrf(String queueName) { queueName)); } - public void handleDynamicMappedQueue(String mapping, boolean create) { - String msg = "Mapping rule %s is dynamic - this might cause inconsistent" - + " behaviour compared to FS."; - - if (create) { - msg += " Also, setting auto-create-child-queue=true is" - + " necessary, because the create flag was set to true on the" - + " original placement rule."; - } - - handle(MAPPED_DYNAMIC_QUEUE, - null, - format(msg, mapping)); - } - private void handle(String actionName, String fsSetting, String message) { RuleAction action = actions.get(actionName); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/QueuePlacementConverter.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/QueuePlacementConverter.java index bfd1b9a5f59..c47dd7615bd 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/QueuePlacementConverter.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/QueuePlacementConverter.java @@ -57,7 +57,7 @@ // nested rule if (userRule.getParentRule() != null) { - handleNestedRule(mapping, userRule, ruleHandler); + handleNestedRule(mapping, userRule); } else { if (!userAsDefaultQueue) { if (mapping.length() > 0) { @@ -102,28 +102,20 @@ } private void handleNestedRule(StringBuilder mapping, - UserPlacementRule userRule, FSConfigToCSConfigRuleHandler ruleHandler) { + UserPlacementRule userRule) { PlacementRule pr = userRule.getParentRule(); if (mapping.length() > 0) { mapping.append(RULE_SEPARATOR); } if (pr instanceof PrimaryGroupPlacementRule) { - String mappingString = "u:" + USER + ":" + PRIMARY_GROUP + "." + USER; - ruleHandler.handleDynamicMappedQueue(mappingString, - ((PrimaryGroupPlacementRule) pr).getCreateFlag()); - mapping.append(mappingString); + mapping.append("u:" + USER + ":" + PRIMARY_GROUP + "." + USER); } else if (pr instanceof SecondaryGroupExistingPlacementRule) { - String mappingString = "u:" + USER + ":" + SECONDARY_GROUP + "." + USER; - ruleHandler.handleDynamicMappedQueue(mappingString, - ((SecondaryGroupExistingPlacementRule) pr).getCreateFlag()); mapping.append("u:" + USER + ":" + SECONDARY_GROUP + "." + USER); } else if (pr instanceof DefaultPlacementRule) { DefaultPlacementRule defaultRule = (DefaultPlacementRule) pr; - String mappingString = - "u:" + USER + ":" + defaultRule.defaultQueueName + "." + USER; - ruleHandler.handleDynamicMappedQueue(mappingString, - defaultRule.getCreateFlag()); - mapping.append(mappingString); + mapping.append("u:" + USER + ":") + .append(defaultRule.defaultQueueName) + .append("." + USER); } else { throw new UnsupportedOperationException("Unsupported nested rule: " + pr.getClass().getCanonicalName()); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSConfigToCSConfigConverter.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSConfigToCSConfigConverter.java index 576025858d5..c229d949159 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSConfigToCSConfigConverter.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestFSConfigToCSConfigConverter.java @@ -26,7 +26,6 @@ import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.converter.FSConfigToCSConfigRuleHandler.USER_MAX_APPS_DEFAULT; import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.converter.FSConfigToCSConfigRuleHandler.USER_MAX_RUNNING_APPS; import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.converter.FSConfigToCSConfigRuleHandler.RuleAction.ABORT; -import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.converter.FSConfigToCSConfigRuleHandler.RuleAction.WARNING; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -373,41 +372,6 @@ public void testConvertFSConfigurationRulesFile() throws Exception { ABORT, actions.get(QUEUE_AUTO_CREATE)); } - @Test - public void testConvertFSConfigurationWithoutRulesFile() throws Exception { - ruleHandler = new FSConfigToCSConfigRuleHandler( - createDefaultConversionOptions()); - createConverter(); - - FSConfigToCSConfigConverterParams params = - createDefaultParamsBuilder() - .withClusterResource(CLUSTER_RESOURCE_STRING) - .build(); - - converter.convert(params); - - ruleHandler = converter.getRuleHandler(); - Map actions = - ruleHandler.getActions(); - - assertEquals("maxCapacityPercentage", - WARNING, actions.get(MAX_CAPACITY_PERCENTAGE)); - assertEquals("maxChildCapacity", - WARNING, actions.get(MAX_CHILD_CAPACITY)); - assertEquals("userMaxRunningApps", - WARNING, actions.get(USER_MAX_RUNNING_APPS)); - assertEquals("userMaxAppsDefault", - WARNING, actions.get(USER_MAX_APPS_DEFAULT)); - assertEquals("dynamicMaxAssign", - WARNING, actions.get(DYNAMIC_MAX_ASSIGN)); - assertEquals("specifiedNotFirstRule", - WARNING, actions.get(SPECIFIED_NOT_FIRST)); - assertEquals("reservationSystem", - WARNING, actions.get(RESERVATION_SYSTEM)); - assertEquals("queueAutoCreate", - WARNING, actions.get(QUEUE_AUTO_CREATE)); - } - @Test public void testConvertFSConfigurationUndefinedYarnSiteConfig() throws Exception { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestQueuePlacementConverter.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestQueuePlacementConverter.java index 1a644bb4c8d..289d35b9499 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestQueuePlacementConverter.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/converter/TestQueuePlacementConverter.java @@ -19,7 +19,6 @@ import static org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacitySchedulerConfiguration.QUEUE_MAPPING; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; @@ -181,8 +180,7 @@ public void testConvertNestedPrimaryGroupRule() { Map properties = convert(false); verifyMapping(properties, "u:%user:%primary_group.%user"); - verify(ruleHandler).handleDynamicMappedQueue( - eq("u:%user:%primary_group.%user"), eq(false)); + verifyZeroInteractions(ruleHandler); } @Test @@ -196,8 +194,7 @@ public void testConvertNestedSecondaryGroupRule() { Map properties = convert(false); verifyMapping(properties, "u:%user:%secondary_group.%user"); - verify(ruleHandler).handleDynamicMappedQueue( - eq("u:%user:%secondary_group.%user"), eq(false)); + verifyZeroInteractions(ruleHandler); } @Test @@ -212,8 +209,7 @@ public void testConvertNestedDefaultRule() { Map properties = convert(false); verifyMapping(properties, "u:%user:abc.%user"); - verify(ruleHandler).handleDynamicMappedQueue( - eq("u:%user:abc.%user"), eq(false)); + verifyZeroInteractions(ruleHandler); } @Test