You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2021/06/01 11:18:30 UTC
[GitHub] [hadoop] AlphaGouGe opened a new pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
AlphaGouGe opened a new pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065
https://issues.apache.org/jira/browse/HDFS-13671
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] kihwal commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
kihwal commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r652143134
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
##########
@@ -3220,21 +3173,28 @@ private void reportDiffSortedInner(
// comes from the IBR / FBR and hence what we should use to compare
// against the memory state.
// See HDFS-6289 and HDFS-15422 for more context.
- queueReportedBlock(storageInfo, replica, reportedState,
+ queueReportedBlock(storageInfo, storedBlock, reportedState,
Review comment:
That's right. This will undo the fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] AlphaGouGe commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
AlphaGouGe commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r645327101
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/DatanodeProtocolClientSideTranslatorPB.java
##########
@@ -188,8 +188,7 @@ public HeartbeatResponse sendHeartbeat(DatanodeRegistration registration,
@Override
public DatanodeCommand blockReport(DatanodeRegistration registration,
- String poolId, StorageBlockReport[] reports,
- BlockReportContext context)
+ String poolId, StorageBlockReport[] reports, BlockReportContext context)
Review comment:
already fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#issuecomment-852363993
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 53s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | buf | 0m 1s | | buf was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 13 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 33m 8s | | trunk passed |
| +1 :green_heart: | compile | 1m 24s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 1m 14s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 1m 7s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 22s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 56s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 26s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 3m 16s | | trunk passed |
| +1 :green_heart: | shadedclient | 18m 45s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 13s | | the patch passed |
| +1 :green_heart: | compile | 1m 18s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | cc | 1m 18s | | the patch passed |
| +1 :green_heart: | javac | 1m 18s | | the patch passed |
| +1 :green_heart: | compile | 1m 8s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | cc | 1m 8s | | the patch passed |
| +1 :green_heart: | javac | 1m 8s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 1m 0s | [/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/1/artifact/out/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt) | hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 1028 unchanged - 13 fixed = 1029 total (was 1041) |
| +1 :green_heart: | mvnsite | 1m 13s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 48s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 16s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 3m 23s | | the patch passed |
| +1 :green_heart: | shadedclient | 19m 5s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 358m 51s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/1/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt) | hadoop-hdfs in the patch passed. |
| -1 :x: | asflicense | 0m 38s | [/results-asflicense.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/1/artifact/out/results-asflicense.txt) | The patch generated 2 ASF License warnings. |
| | | 450m 55s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.tools.TestHdfsConfigFields |
| | hadoop.hdfs.server.namenode.ha.TestBootstrapStandby |
| | hadoop.hdfs.server.namenode.ha.TestEditLogTailer |
| | hadoop.hdfs.TestDFSShell |
| | hadoop.hdfs.TestDecommission |
| | hadoop.hdfs.server.datanode.fsdataset.impl.TestFsVolumeList |
| | hadoop.hdfs.server.namenode.TestDecommissioningStatusWithBackoffMonitor |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3065 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell cc buflint bufcompat |
| uname | Linux 93502d85aa10 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / ee516f731bbbcf5c4ecb4b45527b5ccf525ba2b0 |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/1/testReport/ |
| Max. process+thread count | 2010 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/1/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] aajisaka commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
aajisaka commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r647363773
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
##########
@@ -3111,106 +3042,127 @@ void processFirstBlockReport(
}
}
- private void reportDiffSorted(DatanodeStorageInfo storageInfo,
- Iterable<BlockReportReplica> newReport,
+ private void reportDiff(DatanodeStorageInfo storageInfo,
+ BlockListAsLongs newReport,
Collection<BlockInfoToAdd> toAdd, // add to DatanodeDescriptor
Collection<BlockInfo> toRemove, // remove from DatanodeDescriptor
Collection<Block> toInvalidate, // should be removed from DN
Collection<BlockToMarkCorrupt> toCorrupt, // add to corrupt replicas list
Collection<StatefulBlockInfo> toUC) { // add to under-construction list
- // The blocks must be sorted and the storagenodes blocks must be sorted
- Iterator<BlockInfo> storageBlocksIterator = storageInfo.getBlockIterator();
+ // place a delimiter in the list which separates blocks
+ // that have been reported from those that have not
DatanodeDescriptor dn = storageInfo.getDatanodeDescriptor();
- BlockInfo storageBlock = null;
-
- for (BlockReportReplica replica : newReport) {
-
- long replicaID = replica.getBlockId();
- if (BlockIdManager.isStripedBlockID(replicaID)
- && (!hasNonEcBlockUsingStripedID ||
- !blocksMap.containsBlock(replica))) {
- replicaID = BlockIdManager.convertToStripedID(replicaID);
- }
-
- ReplicaState reportedState = replica.getState();
-
- LOG.debug("Reported block {} on {} size {} replicaState = {}",
- replica, dn, replica.getNumBytes(), reportedState);
-
- if (shouldPostponeBlocksFromFuture
- && isGenStampInFuture(replica)) {
- queueReportedBlock(storageInfo, replica, reportedState,
- QUEUE_REASON_FUTURE_GENSTAMP);
- continue;
- }
-
- if (storageBlock == null && storageBlocksIterator.hasNext()) {
- storageBlock = storageBlocksIterator.next();
- }
-
- do {
- int cmp;
- if (storageBlock == null ||
- (cmp = Long.compare(replicaID, storageBlock.getBlockId())) < 0) {
- // Check if block is available in NN but not yet on this storage
- BlockInfo nnBlock = blocksMap.getStoredBlock(new Block(replicaID));
- if (nnBlock != null) {
- reportDiffSortedInner(storageInfo, replica, reportedState,
- nnBlock, toAdd, toCorrupt, toUC);
- } else {
- // Replica not found anywhere so it should be invalidated
- toInvalidate.add(new Block(replica));
- }
- break;
- } else if (cmp == 0) {
- // Replica matched current storageblock
- reportDiffSortedInner(storageInfo, replica, reportedState,
- storageBlock, toAdd, toCorrupt, toUC);
- storageBlock = null;
- } else {
- // replica has higher ID than storedBlock
- // Remove all stored blocks with IDs lower than replica
- do {
- toRemove.add(storageBlock);
- storageBlock = storageBlocksIterator.hasNext()
- ? storageBlocksIterator.next() : null;
- } while (storageBlock != null &&
- Long.compare(replicaID, storageBlock.getBlockId()) > 0);
+ Block delimiterBlock = new Block();
+ BlockInfo delimiter = new BlockInfoContiguous(delimiterBlock,
+ (short) 1);
+ AddBlockResult result = storageInfo.addBlock(delimiter, delimiterBlock);
+ assert result == AddBlockResult.ADDED
+ : "Delimiting block cannot be present in the node";
+ int headIndex = 0; //currently the delimiter is in the head of the list
+ int curIndex;
+
+ if (newReport == null) {
+ newReport = BlockListAsLongs.EMPTY;
+ }
+ // scan the report and process newly reported blocks
+ for (BlockReportReplica iblk : newReport) {
+ ReplicaState iState = iblk.getState();
+ LOG.debug("Reported block {} on {} size {} replicaState = {}", iblk, dn,
+ iblk.getNumBytes(), iState);
+ BlockInfo storedBlock = processReportedBlock(storageInfo,
+ iblk, iState, toAdd, toInvalidate, toCorrupt, toUC);
+
+ // move block to the head of the list
+ if (storedBlock != null) {
+ curIndex = storedBlock.findStorageInfo(storageInfo);
+ if (curIndex >= 0) {
+ headIndex =
+ storageInfo.moveBlockToHead(storedBlock, curIndex, headIndex);
}
- } while (storageBlock != null);
+ }
}
- // Iterate any remaining blocks that have not been reported and remove them
- while (storageBlocksIterator.hasNext()) {
- toRemove.add(storageBlocksIterator.next());
+ // collect blocks that have not been reported
+ // all of them are next to the delimiter
+ Iterator<BlockInfo> it =
+ storageInfo.new BlockIterator(delimiter.getNext(0));
+ while (it.hasNext()) {
+ toRemove.add(it.next());
}
+ storageInfo.removeBlock(delimiter);
}
- private void reportDiffSortedInner(
+ /**
+ * Process a block replica reported by the data-node.
+ * No side effects except adding to the passed-in Collections.
+ *
+ * <ol>
+ * <li>If the block is not known to the system (not in blocksMap) then the
+ * data-node should be notified to invalidate this block.</li>
+ * <li>If the reported replica is valid that is has the same generation stamp
+ * and length as recorded on the name-node, then the replica location should
+ * be added to the name-node.</li>
+ * <li>If the reported replica is not valid, then it is marked as corrupt,
+ * which triggers replication of the existing valid replicas.
+ * Corrupt replicas are removed from the system when the block
+ * is fully replicated.</li>
+ * <li>If the reported replica is for a block currently marked "under
+ * construction" in the NN, then it should be added to the
+ * BlockUnderConstructionFeature's list of replicas.</li>
+ * </ol>
+ *
+ * @param storageInfo DatanodeStorageInfo that sent the report.
+ * @param block reported block replica
+ * @param reportedState reported replica state
+ * @param toAdd add to DatanodeDescriptor
+ * @param toInvalidate missing blocks (not in the blocks map)
+ * should be removed from the data-node
+ * @param toCorrupt replicas with unexpected length or generation stamp;
+ * add to corrupt replicas
+ * @param toUC replicas of blocks currently under construction
+ * @return the up-to-date stored block, if it should be kept.
+ * Otherwise, null.
+ */
+ private BlockInfo processReportedBlock(
final DatanodeStorageInfo storageInfo,
- final BlockReportReplica replica, final ReplicaState reportedState,
- final BlockInfo storedBlock,
+ final Block block, final ReplicaState reportedState,
final Collection<BlockInfoToAdd> toAdd,
+ final Collection<Block> toInvalidate,
final Collection<BlockToMarkCorrupt> toCorrupt,
final Collection<StatefulBlockInfo> toUC) {
- assert replica != null;
- assert storedBlock != null;
-
DatanodeDescriptor dn = storageInfo.getDatanodeDescriptor();
+
+ LOG.debug("Reported block {} on {} size {} replicaState = {}", block, dn,
Review comment:
Nit: two whitespaces between {} and replicaState.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] kihwal commented on pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
kihwal commented on pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#issuecomment-861825275
I think the patch looks good except for the things that were already pointed out by others.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] aajisaka commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
aajisaka commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r647294150
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/ReplicaMap.java
##########
@@ -238,9 +221,10 @@ ReplicaInfo remove(String bpid, long blockId) {
* @return the number of replicas in the map
*/
int size(String bpid) {
+ LightWeightResizableGSet<Block, ReplicaInfo> m = null;
try (AutoCloseableLock l = readLock.acquire()) {
- FoldedTreeSet<ReplicaInfo> set = map.get(bpid);
- return set != null ? set.size() : 0;
+ m = map.get(bpid);
Review comment:
The definition of `m` can be moved into the try clause.
```suggestion
GSet<Block, ReplicaInfo> m = map.get(bpid);
```
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeStorageInfo.java
##########
@@ -262,30 +257,61 @@ public AddBlockResult addBlock(BlockInfo b, Block reportedBlock) {
}
}
+ // add to the head of the data-node list
b.addStorage(this, reportedBlock);
- blocks.add(b);
+ insertToList(b);
return result;
}
AddBlockResult addBlock(BlockInfo b) {
return addBlock(b, b);
}
- boolean removeBlock(BlockInfo b) {
- blocks.remove(b);
- return b.removeStorage(this);
+ public void insertToList(BlockInfo b) {
+ blockList = b.listInsert(blockList, this);
+ numBlocks++;
+ }
+ public boolean removeBlock(BlockInfo b) {
+ blockList = b.listRemove(blockList, this);
+ if (b.removeStorage(this)) {
+ numBlocks--;
+ return true;
+ } else {
+ return false;
+ }
}
int numBlocks() {
- return blocks.size();
+ return numBlocks;
}
-
+
+ Iterator<BlockInfo> getBlockIterator() {
+ return new BlockIterator(blockList);
+ }
+
/**
- * @return iterator to an unmodifiable set of blocks
- * related to this {@link DatanodeStorageInfo}
+ * Move block to the head of the list of blocks belonging to the data-node.
+ * @return the index of the head of the blockList
*/
- Iterator<BlockInfo> getBlockIterator() {
- return Collections.unmodifiableSet(blocks).iterator();
+ int moveBlockToHead(BlockInfo b, int curIndex, int headIndex) {
+ blockList = b.moveBlockToHead(blockList, this, curIndex, headIndex);
+ return curIndex;
+ }
+
+ int getHeadIndex(DatanodeStorageInfo storageInfo) {
+ if (blockList == null) {
+ return -1;
+ }
+ return blockList.findStorageInfo(storageInfo);
+ }
Review comment:
This function is unused. It can be removed.
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockHasMultipleReplicasOnSameDN.java
##########
@@ -118,13 +116,12 @@ public void testBlockHasMultipleReplicasOnSameDN() throws IOException {
StorageBlockReport reports[] =
new StorageBlockReport[cluster.getStoragesPerDatanode()];
- ArrayList<ReplicaInfo> blocks = new ArrayList<>();
+ ArrayList<Replica> blocks = new ArrayList<Replica>();
Review comment:
Nit:
```suggestion
ArrayList<Replica> blocks = new ArrayList<>();
```
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/proto/DatanodeProtocol.proto
##########
@@ -256,9 +256,6 @@ message BlockReportContextProto {
// The block report lease ID, or 0 if we are sending without a lease to
// bypass rate-limiting.
optional uint64 leaseId = 4 [ default = 0 ];
-
- // True if the reported blocks are sorted by increasing block IDs
- optional bool sorted = 5 [default = false];
Review comment:
We must document that field number 5 cannot be reused.
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
##########
@@ -1996,7 +1996,12 @@ private void metaSave(PrintWriter out) {
LightWeightHashSet<Long> openFileIds = new LightWeightHashSet<>();
for (DatanodeDescriptor dataNode :
blockManager.getDatanodeManager().getDatanodes()) {
- for (long ucFileId : dataNode.getLeavingServiceStatus().getOpenFiles()) {
+ // Sort open files
+ LightWeightHashSet<Long> dnOpenFiles =
+ dataNode.getLeavingServiceStatus().getOpenFiles();
+ Long[] dnOpenFileIds = new Long[dnOpenFiles.size()];
+ Arrays.sort(dnOpenFiles.toArray(dnOpenFileIds));
+ for (Long ucFileId : dnOpenFileIds) {
Review comment:
I thought we have to sort inode ID throughout the DataNodes. However, It can be addressed in a separate jira because it is not sorted throughout the DataNodes even before applying the patch.
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeStorageInfo.java
##########
@@ -262,30 +257,61 @@ public AddBlockResult addBlock(BlockInfo b, Block reportedBlock) {
}
}
+ // add to the head of the data-node list
b.addStorage(this, reportedBlock);
- blocks.add(b);
+ insertToList(b);
return result;
}
AddBlockResult addBlock(BlockInfo b) {
return addBlock(b, b);
}
- boolean removeBlock(BlockInfo b) {
- blocks.remove(b);
- return b.removeStorage(this);
+ public void insertToList(BlockInfo b) {
+ blockList = b.listInsert(blockList, this);
+ numBlocks++;
+ }
+ public boolean removeBlock(BlockInfo b) {
Review comment:
I think `DatanodeStorageInfo#removeBlock` and `ProvidedStorageMap#removeBlock` can be package-private.
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java
##########
@@ -1695,10 +1630,9 @@ private void verifyPlacementPolicy(final MiniDFSCluster cluster,
LocatedBlock lb = DFSTestUtil.getAllBlocks(dfs, file).get(0);
BlockInfo blockInfo =
blockManager.getStoredBlock(lb.getBlock().getLocalBlock());
- Iterator<DatanodeStorageInfo> itr = blockInfo.getStorageInfos();
LOG.info("Block " + blockInfo + " storages: ");
- while (itr.hasNext()) {
- DatanodeStorageInfo dn = itr.next();
+ for (int i = 0; i < 3; i++) {
+ DatanodeStorageInfo dn = blockInfo.getStorageInfo(i);
Review comment:
Is there any specific reason to use `#getStorageInfo(i)` instead of `#getStorageInfos()`? You need to assert the size >= 3?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] ferhui commented on pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
ferhui commented on pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#issuecomment-861097055
@xiaoyuyao Thanks for review! Is @AlphaGouGe 's fix OK?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] aajisaka commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
aajisaka commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r647396463
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
##########
@@ -1996,7 +1996,12 @@ private void metaSave(PrintWriter out) {
LightWeightHashSet<Long> openFileIds = new LightWeightHashSet<>();
for (DatanodeDescriptor dataNode :
blockManager.getDatanodeManager().getDatanodes()) {
- for (long ucFileId : dataNode.getLeavingServiceStatus().getOpenFiles()) {
+ // Sort open files
+ LightWeightHashSet<Long> dnOpenFiles =
+ dataNode.getLeavingServiceStatus().getOpenFiles();
+ Long[] dnOpenFileIds = new Long[dnOpenFiles.size()];
+ Arrays.sort(dnOpenFiles.toArray(dnOpenFileIds));
+ for (Long ucFileId : dnOpenFileIds) {
INode ucFile = getFSDirectory().getInode(ucFileId);
if (ucFile == null || ucFileId <= prevId ||
openFileIds.contains(ucFileId)) {
Review comment:
Agreed. It must be sorted.
I think HDFS-11847 has a bug. If the DataNodes have the following open files and we want to list all the open files:
DN1: [1001, 1002, 1003, ... , 2000]
DN2: [1, 2, 3, ... , 1000]
At first `getFilesBlockingDecom(0, "/")` is called and it returns [1001, 1002, ... , 2000] because it reached max size (=1000), and next `getFilesBlockingDecom(2000, "/")` is called because the last inode Id of the previous result is 2000. That way the open files of DN2 is missed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] aajisaka commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
aajisaka commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r649700823
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
##########
@@ -3111,106 +3042,127 @@ void processFirstBlockReport(
}
}
- private void reportDiffSorted(DatanodeStorageInfo storageInfo,
- Iterable<BlockReportReplica> newReport,
+ private void reportDiff(DatanodeStorageInfo storageInfo,
+ BlockListAsLongs newReport,
Collection<BlockInfoToAdd> toAdd, // add to DatanodeDescriptor
Collection<BlockInfo> toRemove, // remove from DatanodeDescriptor
Collection<Block> toInvalidate, // should be removed from DN
Collection<BlockToMarkCorrupt> toCorrupt, // add to corrupt replicas list
Collection<StatefulBlockInfo> toUC) { // add to under-construction list
- // The blocks must be sorted and the storagenodes blocks must be sorted
- Iterator<BlockInfo> storageBlocksIterator = storageInfo.getBlockIterator();
+ // place a delimiter in the list which separates blocks
+ // that have been reported from those that have not
DatanodeDescriptor dn = storageInfo.getDatanodeDescriptor();
- BlockInfo storageBlock = null;
-
- for (BlockReportReplica replica : newReport) {
-
- long replicaID = replica.getBlockId();
- if (BlockIdManager.isStripedBlockID(replicaID)
- && (!hasNonEcBlockUsingStripedID ||
- !blocksMap.containsBlock(replica))) {
- replicaID = BlockIdManager.convertToStripedID(replicaID);
- }
-
- ReplicaState reportedState = replica.getState();
-
- LOG.debug("Reported block {} on {} size {} replicaState = {}",
- replica, dn, replica.getNumBytes(), reportedState);
-
- if (shouldPostponeBlocksFromFuture
- && isGenStampInFuture(replica)) {
- queueReportedBlock(storageInfo, replica, reportedState,
- QUEUE_REASON_FUTURE_GENSTAMP);
- continue;
- }
-
- if (storageBlock == null && storageBlocksIterator.hasNext()) {
- storageBlock = storageBlocksIterator.next();
- }
-
- do {
- int cmp;
- if (storageBlock == null ||
- (cmp = Long.compare(replicaID, storageBlock.getBlockId())) < 0) {
- // Check if block is available in NN but not yet on this storage
- BlockInfo nnBlock = blocksMap.getStoredBlock(new Block(replicaID));
- if (nnBlock != null) {
- reportDiffSortedInner(storageInfo, replica, reportedState,
- nnBlock, toAdd, toCorrupt, toUC);
- } else {
- // Replica not found anywhere so it should be invalidated
- toInvalidate.add(new Block(replica));
- }
- break;
- } else if (cmp == 0) {
- // Replica matched current storageblock
- reportDiffSortedInner(storageInfo, replica, reportedState,
- storageBlock, toAdd, toCorrupt, toUC);
- storageBlock = null;
- } else {
- // replica has higher ID than storedBlock
- // Remove all stored blocks with IDs lower than replica
- do {
- toRemove.add(storageBlock);
- storageBlock = storageBlocksIterator.hasNext()
- ? storageBlocksIterator.next() : null;
- } while (storageBlock != null &&
- Long.compare(replicaID, storageBlock.getBlockId()) > 0);
+ Block delimiterBlock = new Block();
+ BlockInfo delimiter = new BlockInfoContiguous(delimiterBlock,
+ (short) 1);
+ AddBlockResult result = storageInfo.addBlock(delimiter, delimiterBlock);
+ assert result == AddBlockResult.ADDED
+ : "Delimiting block cannot be present in the node";
+ int headIndex = 0; //currently the delimiter is in the head of the list
+ int curIndex;
+
+ if (newReport == null) {
+ newReport = BlockListAsLongs.EMPTY;
+ }
+ // scan the report and process newly reported blocks
+ for (BlockReportReplica iblk : newReport) {
+ ReplicaState iState = iblk.getState();
+ LOG.debug("Reported block {} on {} size {} replicaState = {}", iblk, dn,
+ iblk.getNumBytes(), iState);
+ BlockInfo storedBlock = processReportedBlock(storageInfo,
+ iblk, iState, toAdd, toInvalidate, toCorrupt, toUC);
+
+ // move block to the head of the list
+ if (storedBlock != null) {
+ curIndex = storedBlock.findStorageInfo(storageInfo);
+ if (curIndex >= 0) {
+ headIndex =
+ storageInfo.moveBlockToHead(storedBlock, curIndex, headIndex);
}
- } while (storageBlock != null);
+ }
}
- // Iterate any remaining blocks that have not been reported and remove them
- while (storageBlocksIterator.hasNext()) {
- toRemove.add(storageBlocksIterator.next());
+ // collect blocks that have not been reported
+ // all of them are next to the delimiter
+ Iterator<BlockInfo> it =
+ storageInfo.new BlockIterator(delimiter.getNext(0));
+ while (it.hasNext()) {
+ toRemove.add(it.next());
}
+ storageInfo.removeBlock(delimiter);
}
- private void reportDiffSortedInner(
+ /**
+ * Process a block replica reported by the data-node.
+ * No side effects except adding to the passed-in Collections.
+ *
+ * <ol>
+ * <li>If the block is not known to the system (not in blocksMap) then the
+ * data-node should be notified to invalidate this block.</li>
+ * <li>If the reported replica is valid that is has the same generation stamp
+ * and length as recorded on the name-node, then the replica location should
+ * be added to the name-node.</li>
+ * <li>If the reported replica is not valid, then it is marked as corrupt,
+ * which triggers replication of the existing valid replicas.
+ * Corrupt replicas are removed from the system when the block
+ * is fully replicated.</li>
+ * <li>If the reported replica is for a block currently marked "under
+ * construction" in the NN, then it should be added to the
+ * BlockUnderConstructionFeature's list of replicas.</li>
+ * </ol>
+ *
+ * @param storageInfo DatanodeStorageInfo that sent the report.
+ * @param block reported block replica
+ * @param reportedState reported replica state
+ * @param toAdd add to DatanodeDescriptor
+ * @param toInvalidate missing blocks (not in the blocks map)
+ * should be removed from the data-node
+ * @param toCorrupt replicas with unexpected length or generation stamp;
+ * add to corrupt replicas
+ * @param toUC replicas of blocks currently under construction
+ * @return the up-to-date stored block, if it should be kept.
+ * Otherwise, null.
+ */
+ private BlockInfo processReportedBlock(
final DatanodeStorageInfo storageInfo,
- final BlockReportReplica replica, final ReplicaState reportedState,
- final BlockInfo storedBlock,
+ final Block block, final ReplicaState reportedState,
final Collection<BlockInfoToAdd> toAdd,
+ final Collection<Block> toInvalidate,
final Collection<BlockToMarkCorrupt> toCorrupt,
final Collection<StatefulBlockInfo> toUC) {
- assert replica != null;
- assert storedBlock != null;
-
DatanodeDescriptor dn = storageInfo.getDatanodeDescriptor();
+
+ LOG.debug("Reported block {} on {} size {} replicaState = {}", block, dn,
Review comment:
If the log level is not debug, toString() is not actually called.
http://www.slf4j.org/faq.html#logging_performance
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] AlphaGouGe commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
AlphaGouGe commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r652450652
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
##########
@@ -3220,21 +3173,28 @@ private void reportDiffSortedInner(
// comes from the IBR / FBR and hence what we should use to compare
// against the memory state.
// See HDFS-6289 and HDFS-15422 for more context.
- queueReportedBlock(storageInfo, replica, reportedState,
+ queueReportedBlock(storageInfo, storedBlock, reportedState,
Review comment:
sorry for missing the change on HDFS-15422, i have changed it back.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] AlphaGouGe commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
AlphaGouGe commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r652451949
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
##########
@@ -3111,106 +3042,127 @@ void processFirstBlockReport(
}
}
- private void reportDiffSorted(DatanodeStorageInfo storageInfo,
- Iterable<BlockReportReplica> newReport,
+ private void reportDiff(DatanodeStorageInfo storageInfo,
+ BlockListAsLongs newReport,
Collection<BlockInfoToAdd> toAdd, // add to DatanodeDescriptor
Collection<BlockInfo> toRemove, // remove from DatanodeDescriptor
Collection<Block> toInvalidate, // should be removed from DN
Collection<BlockToMarkCorrupt> toCorrupt, // add to corrupt replicas list
Collection<StatefulBlockInfo> toUC) { // add to under-construction list
- // The blocks must be sorted and the storagenodes blocks must be sorted
- Iterator<BlockInfo> storageBlocksIterator = storageInfo.getBlockIterator();
+ // place a delimiter in the list which separates blocks
+ // that have been reported from those that have not
DatanodeDescriptor dn = storageInfo.getDatanodeDescriptor();
- BlockInfo storageBlock = null;
-
- for (BlockReportReplica replica : newReport) {
-
- long replicaID = replica.getBlockId();
- if (BlockIdManager.isStripedBlockID(replicaID)
- && (!hasNonEcBlockUsingStripedID ||
- !blocksMap.containsBlock(replica))) {
- replicaID = BlockIdManager.convertToStripedID(replicaID);
- }
-
- ReplicaState reportedState = replica.getState();
-
- LOG.debug("Reported block {} on {} size {} replicaState = {}",
- replica, dn, replica.getNumBytes(), reportedState);
-
- if (shouldPostponeBlocksFromFuture
- && isGenStampInFuture(replica)) {
- queueReportedBlock(storageInfo, replica, reportedState,
- QUEUE_REASON_FUTURE_GENSTAMP);
- continue;
- }
-
- if (storageBlock == null && storageBlocksIterator.hasNext()) {
- storageBlock = storageBlocksIterator.next();
- }
-
- do {
- int cmp;
- if (storageBlock == null ||
- (cmp = Long.compare(replicaID, storageBlock.getBlockId())) < 0) {
- // Check if block is available in NN but not yet on this storage
- BlockInfo nnBlock = blocksMap.getStoredBlock(new Block(replicaID));
- if (nnBlock != null) {
- reportDiffSortedInner(storageInfo, replica, reportedState,
- nnBlock, toAdd, toCorrupt, toUC);
- } else {
- // Replica not found anywhere so it should be invalidated
- toInvalidate.add(new Block(replica));
- }
- break;
- } else if (cmp == 0) {
- // Replica matched current storageblock
- reportDiffSortedInner(storageInfo, replica, reportedState,
- storageBlock, toAdd, toCorrupt, toUC);
- storageBlock = null;
- } else {
- // replica has higher ID than storedBlock
- // Remove all stored blocks with IDs lower than replica
- do {
- toRemove.add(storageBlock);
- storageBlock = storageBlocksIterator.hasNext()
- ? storageBlocksIterator.next() : null;
- } while (storageBlock != null &&
- Long.compare(replicaID, storageBlock.getBlockId()) > 0);
+ Block delimiterBlock = new Block();
+ BlockInfo delimiter = new BlockInfoContiguous(delimiterBlock,
+ (short) 1);
+ AddBlockResult result = storageInfo.addBlock(delimiter, delimiterBlock);
+ assert result == AddBlockResult.ADDED
+ : "Delimiting block cannot be present in the node";
+ int headIndex = 0; //currently the delimiter is in the head of the list
+ int curIndex;
+
+ if (newReport == null) {
+ newReport = BlockListAsLongs.EMPTY;
+ }
+ // scan the report and process newly reported blocks
+ for (BlockReportReplica iblk : newReport) {
+ ReplicaState iState = iblk.getState();
+ LOG.debug("Reported block {} on {} size {} replicaState = {}", iblk, dn,
+ iblk.getNumBytes(), iState);
+ BlockInfo storedBlock = processReportedBlock(storageInfo,
+ iblk, iState, toAdd, toInvalidate, toCorrupt, toUC);
+
+ // move block to the head of the list
+ if (storedBlock != null) {
+ curIndex = storedBlock.findStorageInfo(storageInfo);
+ if (curIndex >= 0) {
+ headIndex =
+ storageInfo.moveBlockToHead(storedBlock, curIndex, headIndex);
}
- } while (storageBlock != null);
+ }
}
- // Iterate any remaining blocks that have not been reported and remove them
- while (storageBlocksIterator.hasNext()) {
- toRemove.add(storageBlocksIterator.next());
+ // collect blocks that have not been reported
+ // all of them are next to the delimiter
+ Iterator<BlockInfo> it =
+ storageInfo.new BlockIterator(delimiter.getNext(0));
+ while (it.hasNext()) {
+ toRemove.add(it.next());
}
+ storageInfo.removeBlock(delimiter);
}
- private void reportDiffSortedInner(
+ /**
+ * Process a block replica reported by the data-node.
+ * No side effects except adding to the passed-in Collections.
+ *
+ * <ol>
+ * <li>If the block is not known to the system (not in blocksMap) then the
+ * data-node should be notified to invalidate this block.</li>
+ * <li>If the reported replica is valid that is has the same generation stamp
+ * and length as recorded on the name-node, then the replica location should
+ * be added to the name-node.</li>
+ * <li>If the reported replica is not valid, then it is marked as corrupt,
+ * which triggers replication of the existing valid replicas.
+ * Corrupt replicas are removed from the system when the block
+ * is fully replicated.</li>
+ * <li>If the reported replica is for a block currently marked "under
+ * construction" in the NN, then it should be added to the
+ * BlockUnderConstructionFeature's list of replicas.</li>
+ * </ol>
+ *
+ * @param storageInfo DatanodeStorageInfo that sent the report.
+ * @param block reported block replica
+ * @param reportedState reported replica state
+ * @param toAdd add to DatanodeDescriptor
+ * @param toInvalidate missing blocks (not in the blocks map)
+ * should be removed from the data-node
+ * @param toCorrupt replicas with unexpected length or generation stamp;
+ * add to corrupt replicas
+ * @param toUC replicas of blocks currently under construction
+ * @return the up-to-date stored block, if it should be kept.
+ * Otherwise, null.
+ */
+ private BlockInfo processReportedBlock(
final DatanodeStorageInfo storageInfo,
- final BlockReportReplica replica, final ReplicaState reportedState,
- final BlockInfo storedBlock,
+ final Block block, final ReplicaState reportedState,
final Collection<BlockInfoToAdd> toAdd,
+ final Collection<Block> toInvalidate,
final Collection<BlockToMarkCorrupt> toCorrupt,
final Collection<StatefulBlockInfo> toUC) {
- assert replica != null;
- assert storedBlock != null;
-
DatanodeDescriptor dn = storageInfo.getDatanodeDescriptor();
+
+ LOG.debug("Reported block {} on {} size {} replicaState = {}", block, dn,
Review comment:
i have changed it back, removing `if (LOG.isDebugEnabled())`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] aajisaka commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
aajisaka commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r648098136
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
##########
@@ -1996,7 +1996,12 @@ private void metaSave(PrintWriter out) {
LightWeightHashSet<Long> openFileIds = new LightWeightHashSet<>();
for (DatanodeDescriptor dataNode :
blockManager.getDatanodeManager().getDatanodes()) {
- for (long ucFileId : dataNode.getLeavingServiceStatus().getOpenFiles()) {
+ // Sort open files
+ LightWeightHashSet<Long> dnOpenFiles =
+ dataNode.getLeavingServiceStatus().getOpenFiles();
+ Long[] dnOpenFileIds = new Long[dnOpenFiles.size()];
+ Arrays.sort(dnOpenFiles.toArray(dnOpenFileIds));
+ for (Long ucFileId : dnOpenFileIds) {
INode ucFile = getFSDirectory().getInode(ucFileId);
if (ucFile == null || ucFileId <= prevId ||
openFileIds.contains(ucFileId)) {
Review comment:
Filed HDFS-16059
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] ferhui commented on pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
ferhui commented on pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#issuecomment-853004590
Failed tests pass locally. Now it is already for review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] ferhui commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
ferhui commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r647902665
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
##########
@@ -1996,7 +1996,12 @@ private void metaSave(PrintWriter out) {
LightWeightHashSet<Long> openFileIds = new LightWeightHashSet<>();
for (DatanodeDescriptor dataNode :
blockManager.getDatanodeManager().getDatanodes()) {
- for (long ucFileId : dataNode.getLeavingServiceStatus().getOpenFiles()) {
+ // Sort open files
+ LightWeightHashSet<Long> dnOpenFiles =
+ dataNode.getLeavingServiceStatus().getOpenFiles();
+ Long[] dnOpenFileIds = new Long[dnOpenFiles.size()];
+ Arrays.sort(dnOpenFiles.toArray(dnOpenFileIds));
+ for (Long ucFileId : dnOpenFileIds) {
INode ucFile = getFSDirectory().getInode(ucFileId);
if (ucFile == null || ucFileId <= prevId ||
openFileIds.contains(ucFileId)) {
Review comment:
@aajisaka Thanks, you are right, it has a bug.
How about keeping the same as before that sort the open files. File a new jira for this bug?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#issuecomment-859499847
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 59s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | buf | 0m 1s | | buf was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 18 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 34m 42s | | trunk passed |
| +1 :green_heart: | compile | 1m 25s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 1m 16s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 1m 8s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 22s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 54s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 25s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 3m 16s | | trunk passed |
| +1 :green_heart: | shadedclient | 18m 55s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 15s | | the patch passed |
| +1 :green_heart: | compile | 1m 18s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | cc | 1m 18s | | the patch passed |
| +1 :green_heart: | javac | 1m 18s | | the patch passed |
| +1 :green_heart: | compile | 1m 8s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | cc | 1m 8s | | the patch passed |
| +1 :green_heart: | javac | 1m 8s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 1m 0s | [/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/6/artifact/out/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt) | hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 1338 unchanged - 13 fixed = 1339 total (was 1351) |
| +1 :green_heart: | mvnsite | 1m 17s | | the patch passed |
| +1 :green_heart: | xml | 0m 1s | | The patch has no ill-formed XML file. |
| +1 :green_heart: | javadoc | 0m 49s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 18s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 3m 20s | | the patch passed |
| +1 :green_heart: | shadedclient | 18m 39s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 346m 2s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/6/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt) | hadoop-hdfs in the patch passed. |
| -1 :x: | asflicense | 0m 38s | [/results-asflicense.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/6/artifact/out/results-asflicense.txt) | The patch generated 2 ASF License warnings. |
| | | 439m 46s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hdfs.server.namenode.ha.TestBootstrapStandby |
| | hadoop.hdfs.server.namenode.ha.TestEditLogTailer |
| | hadoop.hdfs.TestDFSShell |
| | hadoop.hdfs.server.datanode.fsdataset.impl.TestFsVolumeList |
| | hadoop.hdfs.server.namenode.TestDecommissioningStatusWithBackoffMonitor |
| | hadoop.hdfs.server.namenode.TestDecommissioningStatus |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/6/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3065 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell cc buflint bufcompat xml |
| uname | Linux 49dbda01f480 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 6685fbc6f821c4fac484bd176febb6aa6267cca5 |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/6/testReport/ |
| Max. process+thread count | 1748 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/6/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] aajisaka commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
aajisaka commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r647376745
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
##########
@@ -1996,7 +1996,12 @@ private void metaSave(PrintWriter out) {
LightWeightHashSet<Long> openFileIds = new LightWeightHashSet<>();
for (DatanodeDescriptor dataNode :
blockManager.getDatanodeManager().getDatanodes()) {
- for (long ucFileId : dataNode.getLeavingServiceStatus().getOpenFiles()) {
+ // Sort open files
+ LightWeightHashSet<Long> dnOpenFiles =
+ dataNode.getLeavingServiceStatus().getOpenFiles();
+ Long[] dnOpenFileIds = new Long[dnOpenFiles.size()];
+ Arrays.sort(dnOpenFiles.toArray(dnOpenFileIds));
+ for (Long ucFileId : dnOpenFileIds) {
Review comment:
@ferhui Thank you for your comment. The change makes sense to keep the previous behavior.
Now I have a question about the previous (and current) behavior.
https://github.com/apache/hadoop/blob/85517df11ae33ab3a06654d40a1ef4d8eae013e3/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java#L1997-L1998
If there are multiple DataNodes with open files, are the inode IDs really sorted?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] ferhui commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
ferhui commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r643905689
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
##########
@@ -1996,7 +1996,11 @@ private void metaSave(PrintWriter out) {
LightWeightHashSet<Long> openFileIds = new LightWeightHashSet<>();
for (DatanodeDescriptor dataNode :
blockManager.getDatanodeManager().getDatanodes()) {
- for (long ucFileId : dataNode.getLeavingServiceStatus().getOpenFiles()) {
+ // Sort open files
+ LightWeightHashSet<Long> dnOpenFiles = dataNode.getLeavingServiceStatus().getOpenFiles();
Review comment:
checkstyle
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] sodonnel commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
sodonnel commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r651611235
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
##########
@@ -3220,21 +3173,28 @@ private void reportDiffSortedInner(
// comes from the IBR / FBR and hence what we should use to compare
// against the memory state.
// See HDFS-6289 and HDFS-15422 for more context.
- queueReportedBlock(storageInfo, replica, reportedState,
+ queueReportedBlock(storageInfo, storedBlock, reportedState,
Review comment:
I this this change is incorrect - we should be queuing the details reported in the IBR, which I think is "replica" here. This change was made in HDFS-15422.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] AlphaGouGe commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
AlphaGouGe commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r645327101
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/DatanodeProtocolClientSideTranslatorPB.java
##########
@@ -188,8 +188,7 @@ public HeartbeatResponse sendHeartbeat(DatanodeRegistration registration,
@Override
public DatanodeCommand blockReport(DatanodeRegistration registration,
- String poolId, StorageBlockReport[] reports,
- BlockReportContext context)
+ String poolId, StorageBlockReport[] reports, BlockReportContext context)
Review comment:
already fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] AlphaGouGe commented on pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
AlphaGouGe commented on pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#issuecomment-859239871
@xiaoyuyao Thanks for review, i have update this PR, take a look please.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] aajisaka commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
aajisaka commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r647396463
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
##########
@@ -1996,7 +1996,12 @@ private void metaSave(PrintWriter out) {
LightWeightHashSet<Long> openFileIds = new LightWeightHashSet<>();
for (DatanodeDescriptor dataNode :
blockManager.getDatanodeManager().getDatanodes()) {
- for (long ucFileId : dataNode.getLeavingServiceStatus().getOpenFiles()) {
+ // Sort open files
+ LightWeightHashSet<Long> dnOpenFiles =
+ dataNode.getLeavingServiceStatus().getOpenFiles();
+ Long[] dnOpenFileIds = new Long[dnOpenFiles.size()];
+ Arrays.sort(dnOpenFiles.toArray(dnOpenFileIds));
+ for (Long ucFileId : dnOpenFileIds) {
INode ucFile = getFSDirectory().getInode(ucFileId);
if (ucFile == null || ucFileId <= prevId ||
openFileIds.contains(ucFileId)) {
Review comment:
Agreed. It must be sorted.
I think HDFS-11847 has a bug. If the DataNodes have the following open files and we want to list all the open files:
DN1: [1001, 1002, 1003, ... , 2000]
DN2: [1, 2, 3, ... , 1000]
At first `getFilesBlockingDecom(0, "/")` is called and it returns [1001, 1002, ... , 2000] because it reached max size (=1000), and next `getFilesBlockingDecom(2000, "/")` is called and the open files of DN2 is missed because the last inode Id of the previous result is 2000.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] aajisaka commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
aajisaka commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r647951023
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
##########
@@ -1996,7 +1996,12 @@ private void metaSave(PrintWriter out) {
LightWeightHashSet<Long> openFileIds = new LightWeightHashSet<>();
for (DatanodeDescriptor dataNode :
blockManager.getDatanodeManager().getDatanodes()) {
- for (long ucFileId : dataNode.getLeavingServiceStatus().getOpenFiles()) {
+ // Sort open files
+ LightWeightHashSet<Long> dnOpenFiles =
+ dataNode.getLeavingServiceStatus().getOpenFiles();
+ Long[] dnOpenFileIds = new Long[dnOpenFiles.size()];
+ Arrays.sort(dnOpenFiles.toArray(dnOpenFileIds));
+ for (Long ucFileId : dnOpenFileIds) {
INode ucFile = getFSDirectory().getInode(ucFileId);
if (ucFile == null || ucFileId <= prevId ||
openFileIds.contains(ucFileId)) {
Review comment:
Yes, I'm +1 to keep the same as before. I'll file a new jira. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] aajisaka commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
aajisaka commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r647376745
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
##########
@@ -1996,7 +1996,12 @@ private void metaSave(PrintWriter out) {
LightWeightHashSet<Long> openFileIds = new LightWeightHashSet<>();
for (DatanodeDescriptor dataNode :
blockManager.getDatanodeManager().getDatanodes()) {
- for (long ucFileId : dataNode.getLeavingServiceStatus().getOpenFiles()) {
+ // Sort open files
+ LightWeightHashSet<Long> dnOpenFiles =
+ dataNode.getLeavingServiceStatus().getOpenFiles();
+ Long[] dnOpenFileIds = new Long[dnOpenFiles.size()];
+ Arrays.sort(dnOpenFiles.toArray(dnOpenFileIds));
+ for (Long ucFileId : dnOpenFileIds) {
Review comment:
@ferhui Thank you for your comment. The change makes sense to keep the previous behavior.
https://github.com/apache/hadoop/blob/85517df11ae33ab3a06654d40a1ef4d8eae013e3/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java#L1997-L1998
If there are multiple DataNodes with open files, are the inode IDs really sorted?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#issuecomment-852973544
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 52s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | buf | 0m 0s | | buf was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 18 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 33m 9s | | trunk passed |
| +1 :green_heart: | compile | 1m 24s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 1m 15s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 1m 8s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 22s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 55s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 23s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 3m 17s | | trunk passed |
| +1 :green_heart: | shadedclient | 18m 45s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 13s | | the patch passed |
| +1 :green_heart: | compile | 1m 18s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | cc | 1m 18s | | the patch passed |
| +1 :green_heart: | javac | 1m 18s | | the patch passed |
| +1 :green_heart: | compile | 1m 8s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | cc | 1m 8s | | the patch passed |
| +1 :green_heart: | javac | 1m 8s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 1m 2s | [/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/2/artifact/out/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt) | hadoop-hdfs-project/hadoop-hdfs: The patch generated 2 new + 1338 unchanged - 13 fixed = 1340 total (was 1351) |
| +1 :green_heart: | mvnsite | 1m 16s | | the patch passed |
| +1 :green_heart: | xml | 0m 1s | | The patch has no ill-formed XML file. |
| +1 :green_heart: | javadoc | 0m 47s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 21s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 3m 21s | | the patch passed |
| +1 :green_heart: | shadedclient | 18m 55s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 354m 59s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/2/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt) | hadoop-hdfs in the patch passed. |
| -1 :x: | asflicense | 0m 39s | [/results-asflicense.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/2/artifact/out/results-asflicense.txt) | The patch generated 2 ASF License warnings. |
| | | 447m 0s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hdfs.server.namenode.ha.TestBootstrapStandby |
| | hadoop.hdfs.TestDFSShell |
| | hadoop.hdfs.server.datanode.fsdataset.impl.TestFsVolumeList |
| | hadoop.hdfs.server.namenode.TestDecommissioningStatusWithBackoffMonitor |
| | hadoop.hdfs.server.namenode.TestDecommissioningStatus |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3065 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell cc buflint bufcompat xml |
| uname | Linux cdb05ad52f99 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 34c1c50c94722daa8a1da96606fba2fb020bc4c2 |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/2/testReport/ |
| Max. process+thread count | 1920 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/2/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] AlphaGouGe commented on pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
AlphaGouGe commented on pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#issuecomment-858550741
@aajisaka @ferhui Thanks for comments, i have fixed all mentions, take a look please.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#issuecomment-854783064
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 1m 3s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | buf | 0m 1s | | buf was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 18 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 37m 56s | | trunk passed |
| +1 :green_heart: | compile | 1m 38s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 1m 26s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 1m 15s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 36s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 2s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 33s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 3m 47s | | trunk passed |
| +1 :green_heart: | shadedclient | 21m 16s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 25s | | the patch passed |
| +1 :green_heart: | compile | 1m 29s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | cc | 1m 29s | | the patch passed |
| +1 :green_heart: | javac | 1m 29s | | the patch passed |
| +1 :green_heart: | compile | 1m 17s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | cc | 1m 17s | | the patch passed |
| +1 :green_heart: | javac | 1m 17s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 1m 6s | [/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/4/artifact/out/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt) | hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 1338 unchanged - 13 fixed = 1339 total (was 1351) |
| +1 :green_heart: | mvnsite | 1m 26s | | the patch passed |
| +1 :green_heart: | xml | 0m 2s | | The patch has no ill-formed XML file. |
| +1 :green_heart: | javadoc | 0m 54s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 29s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 3m 48s | | the patch passed |
| +1 :green_heart: | shadedclient | 20m 44s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 381m 20s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/4/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt) | hadoop-hdfs in the patch passed. |
| -1 :x: | asflicense | 0m 41s | [/results-asflicense.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/4/artifact/out/results-asflicense.txt) | The patch generated 2 ASF License warnings. |
| | | 485m 4s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hdfs.server.namenode.ha.TestBootstrapStandby |
| | hadoop.hdfs.server.namenode.ha.TestEditLogTailer |
| | hadoop.hdfs.TestDFSShell |
| | hadoop.hdfs.server.datanode.fsdataset.impl.TestFsVolumeList |
| | hadoop.hdfs.server.namenode.TestDecommissioningStatusWithBackoffMonitor |
| | hadoop.hdfs.TestHDFSFileSystemContract |
| | hadoop.hdfs.server.namenode.TestDecommissioningStatus |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3065 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell cc buflint bufcompat xml |
| uname | Linux 103fae16a9dd 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 85517df11ae33ab3a06654d40a1ef4d8eae013e3 |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/4/testReport/ |
| Max. process+thread count | 1878 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/4/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] xiaoyuyao commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r648608033
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
##########
@@ -3220,21 +3172,28 @@ private void reportDiffSortedInner(
// comes from the IBR / FBR and hence what we should use to compare
// against the memory state.
// See HDFS-6289 and HDFS-15422 for more context.
- queueReportedBlock(storageInfo, replica, reportedState,
+ queueReportedBlock(storageInfo, block, reportedState,
Review comment:
block should be storedBlock?
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/proto/DatanodeProtocol.proto
##########
@@ -256,9 +256,6 @@ message BlockReportContextProto {
// The block report lease ID, or 0 if we are sending without a lease to
// bypass rate-limiting.
optional uint64 leaseId = 4 [ default = 0 ];
-
- // True if the reported blocks are sorted by increasing block IDs
- optional bool sorted = 5 [default = false];
Review comment:
should we leave here as-is to keep backward compatibility but leave it unused.
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
##########
@@ -3111,106 +3042,127 @@ void processFirstBlockReport(
}
}
- private void reportDiffSorted(DatanodeStorageInfo storageInfo,
- Iterable<BlockReportReplica> newReport,
+ private void reportDiff(DatanodeStorageInfo storageInfo,
+ BlockListAsLongs newReport,
Collection<BlockInfoToAdd> toAdd, // add to DatanodeDescriptor
Collection<BlockInfo> toRemove, // remove from DatanodeDescriptor
Collection<Block> toInvalidate, // should be removed from DN
Collection<BlockToMarkCorrupt> toCorrupt, // add to corrupt replicas list
Collection<StatefulBlockInfo> toUC) { // add to under-construction list
- // The blocks must be sorted and the storagenodes blocks must be sorted
- Iterator<BlockInfo> storageBlocksIterator = storageInfo.getBlockIterator();
+ // place a delimiter in the list which separates blocks
+ // that have been reported from those that have not
DatanodeDescriptor dn = storageInfo.getDatanodeDescriptor();
- BlockInfo storageBlock = null;
-
- for (BlockReportReplica replica : newReport) {
-
- long replicaID = replica.getBlockId();
- if (BlockIdManager.isStripedBlockID(replicaID)
- && (!hasNonEcBlockUsingStripedID ||
- !blocksMap.containsBlock(replica))) {
- replicaID = BlockIdManager.convertToStripedID(replicaID);
- }
-
- ReplicaState reportedState = replica.getState();
-
- LOG.debug("Reported block {} on {} size {} replicaState = {}",
- replica, dn, replica.getNumBytes(), reportedState);
-
- if (shouldPostponeBlocksFromFuture
- && isGenStampInFuture(replica)) {
- queueReportedBlock(storageInfo, replica, reportedState,
- QUEUE_REASON_FUTURE_GENSTAMP);
- continue;
- }
-
- if (storageBlock == null && storageBlocksIterator.hasNext()) {
- storageBlock = storageBlocksIterator.next();
- }
-
- do {
- int cmp;
- if (storageBlock == null ||
- (cmp = Long.compare(replicaID, storageBlock.getBlockId())) < 0) {
- // Check if block is available in NN but not yet on this storage
- BlockInfo nnBlock = blocksMap.getStoredBlock(new Block(replicaID));
- if (nnBlock != null) {
- reportDiffSortedInner(storageInfo, replica, reportedState,
- nnBlock, toAdd, toCorrupt, toUC);
- } else {
- // Replica not found anywhere so it should be invalidated
- toInvalidate.add(new Block(replica));
- }
- break;
- } else if (cmp == 0) {
- // Replica matched current storageblock
- reportDiffSortedInner(storageInfo, replica, reportedState,
- storageBlock, toAdd, toCorrupt, toUC);
- storageBlock = null;
- } else {
- // replica has higher ID than storedBlock
- // Remove all stored blocks with IDs lower than replica
- do {
- toRemove.add(storageBlock);
- storageBlock = storageBlocksIterator.hasNext()
- ? storageBlocksIterator.next() : null;
- } while (storageBlock != null &&
- Long.compare(replicaID, storageBlock.getBlockId()) > 0);
+ Block delimiterBlock = new Block();
+ BlockInfo delimiter = new BlockInfoContiguous(delimiterBlock,
+ (short) 1);
+ AddBlockResult result = storageInfo.addBlock(delimiter, delimiterBlock);
+ assert result == AddBlockResult.ADDED
+ : "Delimiting block cannot be present in the node";
+ int headIndex = 0; //currently the delimiter is in the head of the list
+ int curIndex;
+
+ if (newReport == null) {
+ newReport = BlockListAsLongs.EMPTY;
+ }
+ // scan the report and process newly reported blocks
+ for (BlockReportReplica iblk : newReport) {
+ ReplicaState iState = iblk.getState();
+ LOG.debug("Reported block {} on {} size {} replicaState = {}", iblk, dn,
+ iblk.getNumBytes(), iState);
+ BlockInfo storedBlock = processReportedBlock(storageInfo,
+ iblk, iState, toAdd, toInvalidate, toCorrupt, toUC);
+
+ // move block to the head of the list
+ if (storedBlock != null) {
+ curIndex = storedBlock.findStorageInfo(storageInfo);
+ if (curIndex >= 0) {
+ headIndex =
+ storageInfo.moveBlockToHead(storedBlock, curIndex, headIndex);
}
- } while (storageBlock != null);
+ }
}
- // Iterate any remaining blocks that have not been reported and remove them
- while (storageBlocksIterator.hasNext()) {
- toRemove.add(storageBlocksIterator.next());
+ // collect blocks that have not been reported
+ // all of them are next to the delimiter
+ Iterator<BlockInfo> it =
+ storageInfo.new BlockIterator(delimiter.getNext(0));
+ while (it.hasNext()) {
+ toRemove.add(it.next());
}
+ storageInfo.removeBlock(delimiter);
}
- private void reportDiffSortedInner(
+ /**
+ * Process a block replica reported by the data-node.
+ * No side effects except adding to the passed-in Collections.
+ *
+ * <ol>
+ * <li>If the block is not known to the system (not in blocksMap) then the
+ * data-node should be notified to invalidate this block.</li>
+ * <li>If the reported replica is valid that is has the same generation stamp
+ * and length as recorded on the name-node, then the replica location should
+ * be added to the name-node.</li>
+ * <li>If the reported replica is not valid, then it is marked as corrupt,
+ * which triggers replication of the existing valid replicas.
+ * Corrupt replicas are removed from the system when the block
+ * is fully replicated.</li>
+ * <li>If the reported replica is for a block currently marked "under
+ * construction" in the NN, then it should be added to the
+ * BlockUnderConstructionFeature's list of replicas.</li>
+ * </ol>
+ *
+ * @param storageInfo DatanodeStorageInfo that sent the report.
+ * @param block reported block replica
+ * @param reportedState reported replica state
+ * @param toAdd add to DatanodeDescriptor
+ * @param toInvalidate missing blocks (not in the blocks map)
+ * should be removed from the data-node
+ * @param toCorrupt replicas with unexpected length or generation stamp;
+ * add to corrupt replicas
+ * @param toUC replicas of blocks currently under construction
+ * @return the up-to-date stored block, if it should be kept.
+ * Otherwise, null.
+ */
+ private BlockInfo processReportedBlock(
final DatanodeStorageInfo storageInfo,
- final BlockReportReplica replica, final ReplicaState reportedState,
- final BlockInfo storedBlock,
+ final Block block, final ReplicaState reportedState,
final Collection<BlockInfoToAdd> toAdd,
+ final Collection<Block> toInvalidate,
final Collection<BlockToMarkCorrupt> toCorrupt,
final Collection<StatefulBlockInfo> toUC) {
- assert replica != null;
- assert storedBlock != null;
-
DatanodeDescriptor dn = storageInfo.getDatanodeDescriptor();
+
+ LOG.debug("Reported block {} on {} size {} replicaState = {}", block, dn,
Review comment:
This needs to be wrapped with if (LOG.isDebugEnabled()) as toString() of dn and block can be expensive.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] ferhui commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
ferhui commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r647357273
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
##########
@@ -1996,7 +1996,12 @@ private void metaSave(PrintWriter out) {
LightWeightHashSet<Long> openFileIds = new LightWeightHashSet<>();
for (DatanodeDescriptor dataNode :
blockManager.getDatanodeManager().getDatanodes()) {
- for (long ucFileId : dataNode.getLeavingServiceStatus().getOpenFiles()) {
+ // Sort open files
+ LightWeightHashSet<Long> dnOpenFiles =
+ dataNode.getLeavingServiceStatus().getOpenFiles();
+ Long[] dnOpenFileIds = new Long[dnOpenFiles.size()];
+ Arrays.sort(dnOpenFiles.toArray(dnOpenFileIds));
+ for (Long ucFileId : dnOpenFileIds) {
Review comment:
@aajisaka Thank you very much for review, add this change because TestDecommission#testDecommissionWithOpenfileReporting fails. In DatanodeAdminDefaultMonitor#processBlocksInternal, blocks are sorted because of FoldedTreeSet, so here inode ids are sorted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] AlphaGouGe commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
AlphaGouGe commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r649666587
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
##########
@@ -3220,21 +3172,28 @@ private void reportDiffSortedInner(
// comes from the IBR / FBR and hence what we should use to compare
// against the memory state.
// See HDFS-6289 and HDFS-15422 for more context.
- queueReportedBlock(storageInfo, replica, reportedState,
+ queueReportedBlock(storageInfo, block, reportedState,
Review comment:
@xiaoyuyao you are right, it should be storedBlock
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] xiaoyuyao commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r644152306
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/DatanodeProtocolClientSideTranslatorPB.java
##########
@@ -188,8 +188,7 @@ public HeartbeatResponse sendHeartbeat(DatanodeRegistration registration,
@Override
public DatanodeCommand blockReport(DatanodeRegistration registration,
- String poolId, StorageBlockReport[] reports,
- BlockReportContext context)
+ String poolId, StorageBlockReport[] reports, BlockReportContext context)
Review comment:
NIT: unnecessary formatting change can be avoided.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] AlphaGouGe commented on pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
AlphaGouGe commented on pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#issuecomment-862150204
Thanks @kihwal @xiaoyuyao @sodonnel for review, i have updated the PR, take a look please.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] aajisaka commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
aajisaka commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r647396463
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
##########
@@ -1996,7 +1996,12 @@ private void metaSave(PrintWriter out) {
LightWeightHashSet<Long> openFileIds = new LightWeightHashSet<>();
for (DatanodeDescriptor dataNode :
blockManager.getDatanodeManager().getDatanodes()) {
- for (long ucFileId : dataNode.getLeavingServiceStatus().getOpenFiles()) {
+ // Sort open files
+ LightWeightHashSet<Long> dnOpenFiles =
+ dataNode.getLeavingServiceStatus().getOpenFiles();
+ Long[] dnOpenFileIds = new Long[dnOpenFiles.size()];
+ Arrays.sort(dnOpenFiles.toArray(dnOpenFileIds));
+ for (Long ucFileId : dnOpenFileIds) {
INode ucFile = getFSDirectory().getInode(ucFileId);
if (ucFile == null || ucFileId <= prevId ||
openFileIds.contains(ucFileId)) {
Review comment:
Agreed. It must be sorted.
I think HDFS-11847 has a bug. If the DataNodes have the following open files and we want to list all the open files:
DN1: [1001, 1002, 1003, ... , 2000]
DN2: [1, 2, 3, ... , 1000]
At first `getFilesBlockingDecom(0, "/")` is called and it returns [1001, 1002, ... , 2000], and next `getFilesBlockingDecom(2000, "/")` is called and the open files of DN2 is missed because the last inode Id of the previous result is 2000.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] ferhui commented on a change in pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
ferhui commented on a change in pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#discussion_r647387834
##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
##########
@@ -1996,7 +1996,12 @@ private void metaSave(PrintWriter out) {
LightWeightHashSet<Long> openFileIds = new LightWeightHashSet<>();
for (DatanodeDescriptor dataNode :
blockManager.getDatanodeManager().getDatanodes()) {
- for (long ucFileId : dataNode.getLeavingServiceStatus().getOpenFiles()) {
+ // Sort open files
+ LightWeightHashSet<Long> dnOpenFiles =
+ dataNode.getLeavingServiceStatus().getOpenFiles();
+ Long[] dnOpenFileIds = new Long[dnOpenFiles.size()];
+ Arrays.sort(dnOpenFiles.toArray(dnOpenFileIds));
+ for (Long ucFileId : dnOpenFileIds) {
INode ucFile = getFSDirectory().getInode(ucFileId);
if (ucFile == null || ucFileId <= prevId ||
openFileIds.contains(ucFileId)) {
Review comment:
@aajisaka Thanks for comments. Previous behavior is that inode ids are sorted for each datanode
Here the if clause (ucFileId <= prevId) is the key point, , it affects remote iterator, if they are not sorted, some open files maybe are missing. This function getFilesBlockingDecom is added by HDFS-11847
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3065: HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3065:
URL: https://github.com/apache/hadoop/pull/3065#issuecomment-858964433
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 22m 21s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | buf | 0m 0s | | buf was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 18 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 33m 30s | | trunk passed |
| +1 :green_heart: | compile | 1m 21s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 1m 14s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 1m 9s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 21s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 55s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 25s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 3m 14s | | trunk passed |
| +1 :green_heart: | shadedclient | 19m 4s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 14s | | the patch passed |
| +1 :green_heart: | compile | 1m 18s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | cc | 1m 18s | | the patch passed |
| +1 :green_heart: | javac | 1m 18s | | the patch passed |
| +1 :green_heart: | compile | 1m 8s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | cc | 1m 8s | | the patch passed |
| +1 :green_heart: | javac | 1m 8s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 1m 1s | [/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/5/artifact/out/results-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt) | hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 1337 unchanged - 13 fixed = 1338 total (was 1350) |
| +1 :green_heart: | mvnsite | 1m 15s | | the patch passed |
| +1 :green_heart: | xml | 0m 1s | | The patch has no ill-formed XML file. |
| +1 :green_heart: | javadoc | 0m 47s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 22s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 3m 20s | | the patch passed |
| +1 :green_heart: | shadedclient | 19m 14s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 357m 17s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/5/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt) | hadoop-hdfs in the patch passed. |
| -1 :x: | asflicense | 0m 37s | [/results-asflicense.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/5/artifact/out/results-asflicense.txt) | The patch generated 2 ASF License warnings. |
| | | 471m 35s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hdfs.server.namenode.ha.TestEditLogTailer |
| | hadoop.hdfs.server.namenode.TestDecommissioningStatusWithBackoffMonitor |
| | hadoop.hdfs.server.namenode.TestDecommissioningStatus |
| | hadoop.hdfs.TestDFSShell |
| | hadoop.hdfs.server.datanode.fsdataset.impl.TestFsVolumeList |
| | hadoop.hdfs.server.namenode.ha.TestBootstrapStandby |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3065 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell cc buflint bufcompat xml |
| uname | Linux 9cf7a549a107 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 5652aae7919e7aa5e5d16ecef2c1baf692881742 |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/5/testReport/ |
| Max. process+thread count | 2014 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3065/5/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org