[GitHub] [ozone] bharatviswa504 opened a new pull request #1531: HDDS-4405. Proxy failover is logging with out trying all OMS.
bharatviswa504 opened a new pull request #1531: URL: https://github.com/apache/ozone/pull/1531 ## What changes were proposed in this pull request? Skip Retry INFO logging on first failover from a proxy is broken. This fixes the behavior. ## What is the link to the Apache JIRA https://issues.apache.org/jira/browse/HDDS-4405 ## How was this patch tested? Tested it on the deployed ozone cluster. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] bharatviswa504 commented on a change in pull request #1503: HDDS-4332: ListFileStatus - do lookup in directory and file tables
bharatviswa504 commented on a change in pull request #1503:
URL: https://github.com/apache/ozone/pull/1503#discussion_r513799166
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##
@@ -2205,6 +2276,318 @@ private void listStatusFindKeyInTableCache(
return fileStatusList;
}
+ public List listStatusV1(OmKeyArgs args, boolean recursive,
+ String startKey, long numEntries, String clientAddress)
+ throws IOException {
+Preconditions.checkNotNull(args, "Key args can not be null");
+
+// unsorted OMKeyInfo list contains combine results from TableCache and DB.
+List fileStatusFinalList = new ArrayList<>();
+LinkedHashSet fileStatusList = new LinkedHashSet<>();
+if (numEntries <= 0) {
+ return fileStatusFinalList;
+}
+
+String volumeName = args.getVolumeName();
+String bucketName = args.getBucketName();
+String keyName = args.getKeyName();
+String seekFileInDB;
+String seekDirInDB;
+long prefixKeyInDB;
+String prefixPath = keyName;
+
+int countEntries = 0;
+
+metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName,
+bucketName);
+try {
+ if (Strings.isNullOrEmpty(startKey)) {
+OzoneFileStatus fileStatus = getFileStatus(args, clientAddress);
+if (fileStatus.isFile()) {
+ return Collections.singletonList(fileStatus);
+}
+
+// Not required to search in DeletedTable because all the deleted
+// keys will be marked directly in dirTable or in keyTable by
+// breaking the pointer to its sub-dirs. So, there is no issue of
+// inconsistency.
+
+/*
+ * keyName is a directory.
+ * Say, "/a" is the dir name and its objectID is 1024, then seek
+ * will be doing with "1024/" to get all immediate descendants.
+ */
+if (fileStatus.getKeyInfo() != null) {
+ prefixKeyInDB = fileStatus.getKeyInfo().getObjectID();
+} else {
+ // list root directory.
+ String bucketKey = metadataManager.getBucketKey(volumeName,
+ bucketName);
+ OmBucketInfo omBucketInfo =
+ metadataManager.getBucketTable().get(bucketKey);
+ prefixKeyInDB = omBucketInfo.getObjectID();
+}
+seekFileInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
+seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
+
+// TODO: recursive flag=true will be handled in HDDS-4360 jira.
+// Order of seek -> (1)Seek dirs in dirTable (2)Seek files in fileTable
+// 1. Seek the given key in key table.
+countEntries = getFilesFromDirectory(fileStatusList, seekFileInDB,
+prefixPath, prefixKeyInDB, startKey, countEntries, numEntries);
+// 2. Seek the given key in dir table.
+getDirectories(recursive, startKey, numEntries, fileStatusList,
+volumeName, bucketName, seekDirInDB, prefixKeyInDB,
+prefixPath, countEntries);
+ } else {
+/*
+ * startKey will be used in iterator seek and sets the beginning point
+ * for key traversal.
+ *
+ * key name will be used as parentID where the user has requested to
+ * list the keys from.
+ *
+ * When recursive flag=false, parentID won't change between two pages.
+ * For example: OM has a namespace like,
+ */a/1...1M files and /a/b/1...1M files.
+ */a/1...1M directories and /a/b/1...1M directories.
+ * Listing "/a", will always have the parentID as "a" irrespective of
+ * the startKey value.
+ */
+// TODO: recursive flag=true will be handled in HDDS-4360 jira.
+OzoneFileStatus fileStatusInfo = getOzoneFileStatusV1(volumeName,
+bucketName, startKey, false, null, true);
+
+if (fileStatusInfo != null) {
+ prefixKeyInDB = fileStatusInfo.getKeyInfo().getParentObjectID();
+ if(fileStatusInfo.isDirectory()){
+seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB,
+fileStatusInfo.getKeyInfo().getFileName());
+
+// Order of seek -> (1) Seek dirs in dirTable. In OM, always the
+// order of search is, first seek into fileTable and then dirTable.
+// So, its not required to search again into the fileTable.
+
+// Seek the given key in dirTable.
+getDirectories(recursive, startKey, numEntries,
+fileStatusList, volumeName, bucketName, seekDirInDB,
+prefixKeyInDB, prefixPath, countEntries);
+
+ } else {
+seekFileInDB = metadataManager.getOzonePathKey(prefixKeyInDB,
+fileStatusInfo.getKeyInfo().getFileName());
+// begins from the first sub-dir under the parent dir
+seekDirInDB
[GitHub] [ozone] xiaoyuyao commented on a change in pull request #1497: HDDS-4345. Replace the deprecated Lock method
xiaoyuyao commented on a change in pull request #1497:
URL: https://github.com/apache/ozone/pull/1497#discussion_r513750621
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##
@@ -459,7 +459,7 @@ public OpenKeySession openKey(OmKeyArgs args) throws
IOException {
args.getVolumeName(), args.getBucketName(), args.getKeyName());
FileEncryptionInfo encInfo;
-metadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volumeName,
+metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName,
Review comment:
This needs a WriteLock.
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/S3SecretManagerImpl.java
##
@@ -61,7 +61,7 @@ public S3SecretValue getS3Secret(String kerberosID) throws
IOException {
Preconditions.checkArgument(Strings.isNotBlank(kerberosID),
"kerberosID cannot be null or empty.");
S3SecretValue result = null;
-omMetadataManager.getLock().acquireWriteLock(S3_SECRET_LOCK, kerberosID);
+omMetadataManager.getLock().acquireReadLock(S3_SECRET_LOCK, kerberosID);
Review comment:
This should be a WriteLock.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] mukul1987 opened a new pull request #1530: HDDS-4363. Add metric to track the number of RocksDB open/close operations.
mukul1987 opened a new pull request #1530: URL: https://github.com/apache/ozone/pull/1530 ## What changes were proposed in this pull request? While benchmarking Ozone performance, it was realized RocksDB open/close operations have impact on performance. Adding metrics on these operations will help understand DataNode performance problems. ## What is the link to the Apache JIRA https://issues.apache.org/jira/browse/HDDS-4363 ## How was this patch tested? Modified TestContainerCache This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] cku328 commented on pull request #1524: HDDS-4258.Set GDPR to a Security submenu in EN and CN document
cku328 commented on pull request #1524: URL: https://github.com/apache/ozone/pull/1524#issuecomment-718114257 Thanks @frischHWC for working on this. When `GDPR in Ozone` is moved from `Features` to `Security`, the description on `Features` page should be updated, IMHO.  This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] avijayanhwx merged pull request #1529: HDDS-4401. Fix compilation issue in HDDS-3698-upgrade branch.
avijayanhwx merged pull request #1529: URL: https://github.com/apache/ozone/pull/1529 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] avijayanhwx commented on pull request #1529: HDDS-4401. Fix compilation issue in HDDS-3698-upgrade branch.
avijayanhwx commented on pull request #1529: URL: https://github.com/apache/ozone/pull/1529#issuecomment-718104100 Integration test failure is unrelated. Since this is a branch PR, merging this with unclean CI to unblock efforts. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] avijayanhwx edited a comment on pull request #1529: HDDS-4401. Fix compilation issue in HDDS-3698-upgrade branch.
avijayanhwx edited a comment on pull request #1529: URL: https://github.com/apache/ozone/pull/1529#issuecomment-718072316 The remaining findbug failures will be fixed through DN finalization JIRA (HDDS-4175). `M D DLS: Dead store to controller in org.apache.hadoop.ozone.container.common.statemachine.commandhandler.FinalizeNewLayoutVersionCommandHandler.handle(SCMCommand, OzoneContainer, StateContext, SCMConnectionManager) At FinalizeNewLayoutVersionCommandHandler.java:[line 73] M D DLS: Dead store to datanodeDetails in org.apache.hadoop.ozone.container.common.statemachine.commandhandler.FinalizeNewLayoutVersionCommandHandler.handle(SCMCommand, OzoneContainer, StateContext, SCMConnectionManager) At FinalizeNewLayoutVersionCommandHandler.java:[line 70] M D DLS: Dead store to finalizeUpgrade in org.apache.hadoop.ozone.container.common.statemachine.commandhandler.FinalizeNewLayoutVersionCommandHandler.handle(SCMCommand, OzoneContainer, StateContext, SCMConnectionManager) At FinalizeNewLayoutVersionCommandHandler.java:[line 75]` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] avijayanhwx commented on pull request #1529: HDDS-4401. Fix compilation issue in HDDS-3698-upgrade branch.
avijayanhwx commented on pull request #1529: URL: https://github.com/apache/ozone/pull/1529#issuecomment-718072316 The current findbug failures will be fixed through DN finalization JIRA (HDDS-4175). `M D DLS: Dead store to controller in org.apache.hadoop.ozone.container.common.statemachine.commandhandler.FinalizeNewLayoutVersionCommandHandler.handle(SCMCommand, OzoneContainer, StateContext, SCMConnectionManager) At FinalizeNewLayoutVersionCommandHandler.java:[line 73] M D DLS: Dead store to datanodeDetails in org.apache.hadoop.ozone.container.common.statemachine.commandhandler.FinalizeNewLayoutVersionCommandHandler.handle(SCMCommand, OzoneContainer, StateContext, SCMConnectionManager) At FinalizeNewLayoutVersionCommandHandler.java:[line 70] M D DLS: Dead store to finalizeUpgrade in org.apache.hadoop.ozone.container.common.statemachine.commandhandler.FinalizeNewLayoutVersionCommandHandler.handle(SCMCommand, OzoneContainer, StateContext, SCMConnectionManager) At FinalizeNewLayoutVersionCommandHandler.java:[line 75]` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] avijayanhwx opened a new pull request #1529: HDDS-4401. Fix compilation issue in HDDS-3698-upgrade branch.
avijayanhwx opened a new pull request #1529: URL: https://github.com/apache/ozone/pull/1529 ## What changes were proposed in this pull request? Fix compilation and findbugs issue. ## What is the link to the Apache JIRA https://issues.apache.org/jira/browse/HDDS-4401 ## How was this patch tested? Checked the branch commit CI run. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] bharatviswa504 commented on pull request #1451: HDDS-4117. Normalize Keypath for listKeys.
bharatviswa504 commented on pull request #1451: URL: https://github.com/apache/ozone/pull/1451#issuecomment-718022766 Thank You @rakeshadr and @elek for the review. @elek if you have any more comments, happy to address them in a new Jira. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] bharatviswa504 merged pull request #1451: HDDS-4117. Normalize Keypath for listKeys.
bharatviswa504 merged pull request #1451: URL: https://github.com/apache/ozone/pull/1451 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] rakeshadr commented on pull request #1451: HDDS-4117. Normalize Keypath for listKeys.
rakeshadr commented on pull request #1451: URL: https://github.com/apache/ozone/pull/1451#issuecomment-718008243 +1 LGTM Thanks @bharatviswa504 for the contribution. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] avijayanhwx commented on pull request #1518: HDDS-4386: Each EndpointStateMachine uses its own thread pool to talk with SCM/Recon
avijayanhwx commented on pull request #1518: URL: https://github.com/apache/ozone/pull/1518#issuecomment-717955429 @GlenGeng / @ChenSammi How many containers do you have on your prod cluster? I have encountered a similar issue in another cluster with million containers. The problem is explained in HDDS-4403. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] rakeshadr opened a new pull request #1528: HDDS-4357: Rename : make rename an atomic ops by updating key path entry in dir/file table
rakeshadr opened a new pull request #1528: URL: https://github.com/apache/ozone/pull/1528 ## What changes were proposed in this pull request? https://issues.apache.org/jira/browse/HDDS-4357 ## What is the link to the Apache JIRA This task is to handle rename key path request and make it an atomic operation by updating the DirTable or FileTable. ## How was this patch tested? Added TestOzoneFileSystem UT. Note: Few refactoring has to be done once HDDS-4332 getFileStatus/ListStatus API is committed to the branch. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] bshashikant opened a new pull request #1527: HDDS-4400. Make raft log directory deletion configurable during pipeline remove.
bshashikant opened a new pull request #1527: URL: https://github.com/apache/ozone/pull/1527 ## What changes were proposed in this pull request? Added a config to make raft log directory removal configurable during pipeline remove. ## What is the link to the Apache JIRA https://issues.apache.org/jira/browse/HDDS-4400 ## How was this patch tested? config change. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] bshashikant opened a new pull request #1526: HDDS-4399. Safe mode rule for piplelines should only consider open pipelines.
bshashikant opened a new pull request #1526: URL: https://github.com/apache/ozone/pull/1526 ## What changes were proposed in this pull request? Currently, for safe mode we consider all pipelines existing in DB for safe mode exit criteria. It ma happen that, SCM has the pipelines craeted , but none of the participants datanodes ever created these datanodes. In such cases, SCM fails to come out of safemode as these pipelines are never reported back to SCM. The idea here is to consider pipelines which are marked open during SCM startup. ## What is the link to the Apache JIRA https://issues.apache.org/jira/browse/HDDS-4399 ## How was this patch tested? Added unit tests This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] linyiqun commented on a change in pull request #1489: HDDS-4308. Fix issue with quota update
linyiqun commented on a change in pull request #1489:
URL: https://github.com/apache/ozone/pull/1489#discussion_r51435
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##
@@ -597,27 +596,40 @@ protected boolean checkDirectoryAlreadyExists(String
volumeName,
}
/**
- * Return volume info for the specified volume. If the volume does not
- * exist, returns {@code null}.
+ * Return volume info that updated usageBytes for the specified volume.
* @param omMetadataManager
* @param volume
+ * @param updateUsage
* @return OmVolumeArgs
* @throws IOException
*/
- protected OmVolumeArgs getVolumeInfo(OMMetadataManager omMetadataManager,
- String volume) {
-
-OmVolumeArgs volumeArgs = null;
-
-CacheValue value =
-omMetadataManager.getVolumeTable().getCacheValue(
-new CacheKey<>(omMetadataManager.getVolumeKey(volume)));
-
-if (value != null) {
- volumeArgs = value.getCacheValue();
-}
+ protected static synchronized OmVolumeArgs syncUpdateUsage(
+ OMMetadataManager omMetadataManager, String volume, long updateUsage) {
+OmVolumeArgs volumeArgs = omMetadataManager.getVolumeTable().getCacheValue(
+new CacheKey<>(omMetadataManager.getVolumeKey(volume)))
+.getCacheValue();
+volumeArgs.getUsedBytes().add(updateUsage);
+return volumeArgs.copyObject();
+ }
-return volumeArgs;
+ /**
+ * Return volume info that updated usageBytes for the specified volume. And
+ * check Volume usageBytes quota.
+ * @param omMetadataManager
+ * @param volume
+ * @param updateUsage
+ * @return OmVolumeArgs
+ * @throws IOException
+ */
+ protected static synchronized OmVolumeArgs syncCheckAndUpdateUsage(
Review comment:
>Thank for @bharatviswa504's feedback. This is a possible case. Can lead
to update usageBytes inaccurate.
Suppose that ThreadA update VolumeArgs using OMVolumeSetOwnerRequest, it
gets VolumeArgs from DB if usageBytes is 1000 at this point. When ThreadB
writes the new key using OMKeyCreateRequest, he changes usageBytes to 1100.
Good catch, @captainzmc . Seems we still need to use volume lock to help us
do the volume quota update control. We should acquire volume lock before get
volume info (copy volume info) and release volume lock after add the
OMClientResponse into double buffer cache.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] linyiqun commented on a change in pull request #1489: HDDS-4308. Fix issue with quota update
linyiqun commented on a change in pull request #1489:
URL: https://github.com/apache/ozone/pull/1489#discussion_r51435
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##
@@ -597,27 +596,40 @@ protected boolean checkDirectoryAlreadyExists(String
volumeName,
}
/**
- * Return volume info for the specified volume. If the volume does not
- * exist, returns {@code null}.
+ * Return volume info that updated usageBytes for the specified volume.
* @param omMetadataManager
* @param volume
+ * @param updateUsage
* @return OmVolumeArgs
* @throws IOException
*/
- protected OmVolumeArgs getVolumeInfo(OMMetadataManager omMetadataManager,
- String volume) {
-
-OmVolumeArgs volumeArgs = null;
-
-CacheValue value =
-omMetadataManager.getVolumeTable().getCacheValue(
-new CacheKey<>(omMetadataManager.getVolumeKey(volume)));
-
-if (value != null) {
- volumeArgs = value.getCacheValue();
-}
+ protected static synchronized OmVolumeArgs syncUpdateUsage(
+ OMMetadataManager omMetadataManager, String volume, long updateUsage) {
+OmVolumeArgs volumeArgs = omMetadataManager.getVolumeTable().getCacheValue(
+new CacheKey<>(omMetadataManager.getVolumeKey(volume)))
+.getCacheValue();
+volumeArgs.getUsedBytes().add(updateUsage);
+return volumeArgs.copyObject();
+ }
-return volumeArgs;
+ /**
+ * Return volume info that updated usageBytes for the specified volume. And
+ * check Volume usageBytes quota.
+ * @param omMetadataManager
+ * @param volume
+ * @param updateUsage
+ * @return OmVolumeArgs
+ * @throws IOException
+ */
+ protected static synchronized OmVolumeArgs syncCheckAndUpdateUsage(
Review comment:
>Thank for @bharatviswa504's feedback. This is a possible case. Can lead
to update usageBytes inaccurate.
Suppose that ThreadA update VolumeArgs using OMVolumeSetOwnerRequest, it
gets VolumeArgs from DB if usageBytes is 1000 at this point. When ThreadB
writes the new key using OMKeyCreateRequest, he changes usageBytes to 1100.
Good catch, @captainzmc . Seems we still use volume lock to help us do the
volume quota update control. We should acquire volume lock before get volume
info (copy volume info) and release volume lock after add the OMClientResponse
into double buffer cache.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] captainzmc commented on a change in pull request #1515: HDDS-4373. [Design] Ozone support append operation
captainzmc commented on a change in pull request #1515: URL: https://github.com/apache/ozone/pull/1515#discussion_r513230594 ## File path: hadoop-hdds/docs/content/design/append.md ## @@ -0,0 +1,87 @@ +--- +title: Append +summary: Append to the existing key. +date: 2020-10-22 +jira: HDDS-4333 +status: implementing +author: captainzmc +--- + + +## Introduction +This is a proposal to introduce append operation for Ozone, which will allow write data in the tail of an existing file. + +## Goals + OzoneClient and OzoneFS Client support append operation. + While the original key is appended to the write, the key needs to be readable by other clients. + After the OutputStream of the new Append operation calls close, other clients can read the new Append content. This ensures consistency of read operations. +## Non-goals +The operation of hflush is not within the scope of this design. Created HDDS-4353 to discuss this. +## Related jira +https://issues.apache.org/jira/browse/HDDS-4333 +## Implementation +### Background conditions： +We can't currently open a closed Container. If append generates a new block every time, the key may have many smaller blocks less than 256MB(Default block size). Too many blocks will make the DB larger and also have an impact on read performance. + +### Solution： +When Append occurs, determine if the container for the last block is closed. If it's closed, we create a new block. if it's open we append data to the last block. This can avoid creating new blocks as much as possible. + +### Request process： + + + 1. Client executes append key operation to OM + + 2. OM checks if the key is in appendTable; if so, the key is being called by another client append. we cannot append this key at this point. If not, add the key to appendTable. + + 3. Check whether the last block of the key belongs to a closed container, if so, apply to SCM allocate a new block, if not, use the current block directly. Review comment: Thanks for @arp7's review. Does changing an existing block cause any other problems? As far as I know, we can't change the block under the closed container because we can't try to reopen the container.However, the block under the container that is not closed can be modified. The purpose of this is to reduce the number of key blocks. If a key Append is particularly frequent, each append will generate a new block. And then finally this key is going to produce a bunch of little blocks. Too many little blocks will make the DB larger and also have an impact on read performance. The current [ozone truncate design](https://github.com/apache/ozone/pull/1504/files) also needs to modify the existing block. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] captainzmc commented on a change in pull request #1515: HDDS-4373. [Design] Ozone support append operation
captainzmc commented on a change in pull request #1515: URL: https://github.com/apache/ozone/pull/1515#discussion_r513229806 ## File path: hadoop-hdds/docs/content/design/append.md ## @@ -0,0 +1,87 @@ +--- +title: Append +summary: Append to the existing key. +date: 2020-10-22 +jira: HDDS-4333 +status: implementing +author: captainzmc +--- + + +## Introduction +This is a proposal to introduce append operation for Ozone, which will allow write data in the tail of an existing file. + +## Goals + OzoneClient and OzoneFS Client support append operation. + While the original key is appended to the write, the key needs to be readable by other clients. + After the OutputStream of the new Append operation calls close, other clients can read the new Append content. This ensures consistency of read operations. +## Non-goals +The operation of hflush is not within the scope of this design. Created HDDS-4353 to discuss this. +## Related jira +https://issues.apache.org/jira/browse/HDDS-4333 +## Implementation +### Background conditions： +We can't currently open a closed Container. If append generates a new block every time, the key may have many smaller blocks less than 256MB(Default block size). Too many blocks will make the DB larger and also have an impact on read performance. + +### Solution： +When Append occurs, determine if the container for the last block is closed. If it's closed, we create a new block. if it's open we append data to the last block. This can avoid creating new blocks as much as possible. + +### Request process： + + + 1. Client executes append key operation to OM + + 2. OM checks if the key is in appendTable; if so, the key is being called by another client append. we cannot append this key at this point. If not, add the key to appendTable. + + 3. Check whether the last block of the key belongs to a closed container, if so, apply to SCM allocate a new block, if not, use the current block directly. Review comment: Thanks for @arp7's review. Does changing an existing block cause any other problems? As far as I know, we can't change the block under the closed container because we can't try to reopen the container.However, the block under the container that is not closed can be modified. The purpose of this is to reduce the number of key blocks. If a key Append is particularly frequent, each append will generate a new block. And then finally this key is going to produce a bunch of little blocks. Too many little blocks will make the DB larger and also have an impact on read performance. The current [ozone truncate design](https://github.com/apache/ozone/pull/1504/files) also needs to modify the existing block. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] captainzmc commented on a change in pull request #1515: HDDS-4373. [Design] Ozone support append operation
captainzmc commented on a change in pull request #1515: URL: https://github.com/apache/ozone/pull/1515#discussion_r513229806 ## File path: hadoop-hdds/docs/content/design/append.md ## @@ -0,0 +1,87 @@ +--- +title: Append +summary: Append to the existing key. +date: 2020-10-22 +jira: HDDS-4333 +status: implementing +author: captainzmc +--- + + +## Introduction +This is a proposal to introduce append operation for Ozone, which will allow write data in the tail of an existing file. + +## Goals + OzoneClient and OzoneFS Client support append operation. + While the original key is appended to the write, the key needs to be readable by other clients. + After the OutputStream of the new Append operation calls close, other clients can read the new Append content. This ensures consistency of read operations. +## Non-goals +The operation of hflush is not within the scope of this design. Created HDDS-4353 to discuss this. +## Related jira +https://issues.apache.org/jira/browse/HDDS-4333 +## Implementation +### Background conditions： +We can't currently open a closed Container. If append generates a new block every time, the key may have many smaller blocks less than 256MB(Default block size). Too many blocks will make the DB larger and also have an impact on read performance. + +### Solution： +When Append occurs, determine if the container for the last block is closed. If it's closed, we create a new block. if it's open we append data to the last block. This can avoid creating new blocks as much as possible. + +### Request process： + + + 1. Client executes append key operation to OM + + 2. OM checks if the key is in appendTable; if so, the key is being called by another client append. we cannot append this key at this point. If not, add the key to appendTable. + + 3. Check whether the last block of the key belongs to a closed container, if so, apply to SCM allocate a new block, if not, use the current block directly. Review comment: Thanks for @arp7's review. Does changing an existing block cause any other problems? As far as I know, we can't change the block under the closed container because we can't try to reopen the container.However, the block under the container that is not closed can be modified. The purpose of this is to reduce the number of key blocks. If a key Append is particularly frequent, each append will generate a new block. And then finally this key is going to produce a bunch of little blocks. Too many little blocks will make the DB larger and also have an impact on read performance. The current [truncate design](https://github.com/apache/ozone/pull/1504/files) also needs to modify the existing block. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] captainzmc commented on a change in pull request #1515: HDDS-4373. [Design] Ozone support append operation
captainzmc commented on a change in pull request #1515: URL: https://github.com/apache/ozone/pull/1515#discussion_r513224086 ## File path: hadoop-hdds/docs/content/design/append.md ## @@ -0,0 +1,87 @@ +--- +title: Append +summary: Append to the existing key. +date: 2020-10-22 +jira: HDDS-4333 +status: implementing +author: captainzmc +--- + + +## Introduction +This is a proposal to introduce append operation for Ozone, which will allow write data in the tail of an existing file. + +## Goals + OzoneClient and OzoneFS Client support append operation. + While the original key is appended to the write, the key needs to be readable by other clients. + After the OutputStream of the new Append operation calls close, other clients can read the new Append content. This ensures consistency of read operations. +## Non-goals +The operation of hflush is not within the scope of this design. Created HDDS-4353 to discuss this. +## Related jira +https://issues.apache.org/jira/browse/HDDS-4333 +## Implementation +### Background conditions： +We can't currently open a closed Container. If append generates a new block every time, the key may have many smaller blocks less than 256MB(Default block size). Too many blocks will make the DB larger and also have an impact on read performance. + +### Solution： +When Append occurs, determine if the container for the last block is closed. If it's closed, we create a new block. if it's open we append data to the last block. This can avoid creating new blocks as much as possible. + +### Request process： + + + 1. Client executes append key operation to OM + + 2. OM checks if the key is in appendTable; if so, the key is being called by another client append. we cannot append this key at this point. If not, add the key to appendTable. Review comment: This similar to implementing a lock for the current Append key. If thread A is in the append key(it get the append lock for the key), thread B cannot append. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] captainzmc commented on a change in pull request #1515: HDDS-4373. [Design] Ozone support append operation
captainzmc commented on a change in pull request #1515: URL: https://github.com/apache/ozone/pull/1515#discussion_r513224086 ## File path: hadoop-hdds/docs/content/design/append.md ## @@ -0,0 +1,87 @@ +--- +title: Append +summary: Append to the existing key. +date: 2020-10-22 +jira: HDDS-4333 +status: implementing +author: captainzmc +--- + + +## Introduction +This is a proposal to introduce append operation for Ozone, which will allow write data in the tail of an existing file. + +## Goals + OzoneClient and OzoneFS Client support append operation. + While the original key is appended to the write, the key needs to be readable by other clients. + After the OutputStream of the new Append operation calls close, other clients can read the new Append content. This ensures consistency of read operations. +## Non-goals +The operation of hflush is not within the scope of this design. Created HDDS-4353 to discuss this. +## Related jira +https://issues.apache.org/jira/browse/HDDS-4333 +## Implementation +### Background conditions： +We can't currently open a closed Container. If append generates a new block every time, the key may have many smaller blocks less than 256MB(Default block size). Too many blocks will make the DB larger and also have an impact on read performance. + +### Solution： +When Append occurs, determine if the container for the last block is closed. If it's closed, we create a new block. if it's open we append data to the last block. This can avoid creating new blocks as much as possible. + +### Request process： + + + 1. Client executes append key operation to OM + + 2. OM checks if the key is in appendTable; if so, the key is being called by another client append. we cannot append this key at this point. If not, add the key to appendTable. Review comment: Thanks for @arp7's review. This similar to implementing a lock for the current Append key. If thread A is in the append key(it get the append lock for the key), thread B cannot append. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] captainzmc commented on a change in pull request #1489: HDDS-4308. Fix issue with quota update
captainzmc commented on a change in pull request #1489:
URL: https://github.com/apache/ozone/pull/1489#discussion_r513216585
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##
@@ -597,27 +596,40 @@ protected boolean checkDirectoryAlreadyExists(String
volumeName,
}
/**
- * Return volume info for the specified volume. If the volume does not
- * exist, returns {@code null}.
+ * Return volume info that updated usageBytes for the specified volume.
* @param omMetadataManager
* @param volume
+ * @param updateUsage
* @return OmVolumeArgs
* @throws IOException
*/
- protected OmVolumeArgs getVolumeInfo(OMMetadataManager omMetadataManager,
- String volume) {
-
-OmVolumeArgs volumeArgs = null;
-
-CacheValue value =
-omMetadataManager.getVolumeTable().getCacheValue(
-new CacheKey<>(omMetadataManager.getVolumeKey(volume)));
-
-if (value != null) {
- volumeArgs = value.getCacheValue();
-}
+ protected static synchronized OmVolumeArgs syncUpdateUsage(
+ OMMetadataManager omMetadataManager, String volume, long updateUsage) {
+OmVolumeArgs volumeArgs = omMetadataManager.getVolumeTable().getCacheValue(
+new CacheKey<>(omMetadataManager.getVolumeKey(volume)))
+.getCacheValue();
+volumeArgs.getUsedBytes().add(updateUsage);
+return volumeArgs.copyObject();
+ }
-return volumeArgs;
+ /**
+ * Return volume info that updated usageBytes for the specified volume. And
+ * check Volume usageBytes quota.
+ * @param omMetadataManager
+ * @param volume
+ * @param updateUsage
+ * @return OmVolumeArgs
+ * @throws IOException
+ */
+ protected static synchronized OmVolumeArgs syncCheckAndUpdateUsage(
Review comment:
> I have not understood one part, how without volume lock this will help
here.
>
> Because other threads can be updating volumeArgs when this update is
happening/ other threads read omVolumeArgs.
Thank for @bharatviswa504's feedback. This is a possible case. Can lead to
update usageBytes inaccurate.
Suppose that ThreadA update VolumeArgs using OMVolumeSetOwnerRequest, it
gets VolumeArgs from DB if usageBytes is 1000 at this point. When ThreadB
writes the new key using OMKeyCreateRequest, he changes usageBytes to 1100.
There is no Volume lock at this point, so ThreadB may finish executing first
and ThreadA last. The Final usageBytes for VolumeArgs would be 1000. This
should be a problem.
Maybe we still can't avoid using Volume lock. hi @linyiqun Any suggestions
here?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] captainzmc commented on a change in pull request #1489: HDDS-4308. Fix issue with quota update
captainzmc commented on a change in pull request #1489:
URL: https://github.com/apache/ozone/pull/1489#discussion_r513219358
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##
@@ -597,27 +596,40 @@ protected boolean checkDirectoryAlreadyExists(String
volumeName,
}
/**
- * Return volume info for the specified volume. If the volume does not
- * exist, returns {@code null}.
+ * Return volume info that updated usageBytes for the specified volume.
* @param omMetadataManager
* @param volume
+ * @param updateUsage
* @return OmVolumeArgs
* @throws IOException
*/
- protected OmVolumeArgs getVolumeInfo(OMMetadataManager omMetadataManager,
- String volume) {
-
-OmVolumeArgs volumeArgs = null;
-
-CacheValue value =
-omMetadataManager.getVolumeTable().getCacheValue(
-new CacheKey<>(omMetadataManager.getVolumeKey(volume)));
-
-if (value != null) {
- volumeArgs = value.getCacheValue();
-}
+ protected static synchronized OmVolumeArgs syncUpdateUsage(
+ OMMetadataManager omMetadataManager, String volume, long updateUsage) {
+OmVolumeArgs volumeArgs = omMetadataManager.getVolumeTable().getCacheValue(
+new CacheKey<>(omMetadataManager.getVolumeKey(volume)))
+.getCacheValue();
+volumeArgs.getUsedBytes().add(updateUsage);
+return volumeArgs.copyObject();
+ }
-return volumeArgs;
+ /**
+ * Return volume info that updated usageBytes for the specified volume. And
+ * check Volume usageBytes quota.
+ * @param omMetadataManager
+ * @param volume
+ * @param updateUsage
+ * @return OmVolumeArgs
+ * @throws IOException
+ */
+ protected static synchronized OmVolumeArgs syncCheckAndUpdateUsage(
Review comment:
> Do we need LongAdder still?
LongAdder will no longer be necessary if there are no concurrent updates to
usedBytes.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] captainzmc commented on a change in pull request #1489: HDDS-4308. Fix issue with quota update
captainzmc commented on a change in pull request #1489:
URL: https://github.com/apache/ozone/pull/1489#discussion_r513219195
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##
@@ -597,27 +596,40 @@ protected boolean checkDirectoryAlreadyExists(String
volumeName,
}
/**
- * Return volume info for the specified volume. If the volume does not
- * exist, returns {@code null}.
+ * Return volume info that updated usageBytes for the specified volume.
* @param omMetadataManager
* @param volume
+ * @param updateUsage
* @return OmVolumeArgs
* @throws IOException
*/
- protected OmVolumeArgs getVolumeInfo(OMMetadataManager omMetadataManager,
- String volume) {
-
-OmVolumeArgs volumeArgs = null;
-
-CacheValue value =
-omMetadataManager.getVolumeTable().getCacheValue(
-new CacheKey<>(omMetadataManager.getVolumeKey(volume)));
-
-if (value != null) {
- volumeArgs = value.getCacheValue();
-}
+ protected static synchronized OmVolumeArgs syncUpdateUsage(
+ OMMetadataManager omMetadataManager, String volume, long updateUsage) {
+OmVolumeArgs volumeArgs = omMetadataManager.getVolumeTable().getCacheValue(
+new CacheKey<>(omMetadataManager.getVolumeKey(volume)))
+.getCacheValue();
+volumeArgs.getUsedBytes().add(updateUsage);
+return volumeArgs.copyObject();
+ }
-return volumeArgs;
+ /**
+ * Return volume info that updated usageBytes for the specified volume. And
+ * check Volume usageBytes quota.
+ * @param omMetadataManager
+ * @param volume
+ * @param updateUsage
+ * @return OmVolumeArgs
+ * @throws IOException
+ */
+ protected static synchronized OmVolumeArgs syncCheckAndUpdateUsage(
Review comment:
> Do we need LongAdder still?
LongAdder will no longer be necessary if there are no concurrent updates to
usedBytes.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] captainzmc commented on a change in pull request #1489: HDDS-4308. Fix issue with quota update
captainzmc commented on a change in pull request #1489:
URL: https://github.com/apache/ozone/pull/1489#discussion_r513216585
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##
@@ -597,27 +596,40 @@ protected boolean checkDirectoryAlreadyExists(String
volumeName,
}
/**
- * Return volume info for the specified volume. If the volume does not
- * exist, returns {@code null}.
+ * Return volume info that updated usageBytes for the specified volume.
* @param omMetadataManager
* @param volume
+ * @param updateUsage
* @return OmVolumeArgs
* @throws IOException
*/
- protected OmVolumeArgs getVolumeInfo(OMMetadataManager omMetadataManager,
- String volume) {
-
-OmVolumeArgs volumeArgs = null;
-
-CacheValue value =
-omMetadataManager.getVolumeTable().getCacheValue(
-new CacheKey<>(omMetadataManager.getVolumeKey(volume)));
-
-if (value != null) {
- volumeArgs = value.getCacheValue();
-}
+ protected static synchronized OmVolumeArgs syncUpdateUsage(
+ OMMetadataManager omMetadataManager, String volume, long updateUsage) {
+OmVolumeArgs volumeArgs = omMetadataManager.getVolumeTable().getCacheValue(
+new CacheKey<>(omMetadataManager.getVolumeKey(volume)))
+.getCacheValue();
+volumeArgs.getUsedBytes().add(updateUsage);
+return volumeArgs.copyObject();
+ }
-return volumeArgs;
+ /**
+ * Return volume info that updated usageBytes for the specified volume. And
+ * check Volume usageBytes quota.
+ * @param omMetadataManager
+ * @param volume
+ * @param updateUsage
+ * @return OmVolumeArgs
+ * @throws IOException
+ */
+ protected static synchronized OmVolumeArgs syncCheckAndUpdateUsage(
Review comment:
> I have not understood one part, how without volume lock this will help
here.
>
> Because other threads can be updating volumeArgs when this update is
happening/ other threads read omVolumeArgs.
Thank for @bharatviswa504's feedback. This is a possible case. Can lead to
update usageBytes inaccurate.
Suppose that ThreadA update VolumeArgs using OMVolumeSetOwnerRequest, it
gets VolumeArgs from DB if usageBytes is 1000 at this point. When ThreadB
writes the new key using OMKeyCreateRequest, he changes usageBytes to 1100.
There is no Volume lock at this point, so ThreadB may finish executing first
and ThreadA last. The Final usageBytes for VolumeArgs would be 1000. This
should be a problem.
Maybe we still can't avoid using Volume lock. Any suggestions here?
@linyiqun
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] captainzmc commented on a change in pull request #1489: HDDS-4308. Fix issue with quota update
captainzmc commented on a change in pull request #1489:
URL: https://github.com/apache/ozone/pull/1489#discussion_r513216585
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##
@@ -597,27 +596,40 @@ protected boolean checkDirectoryAlreadyExists(String
volumeName,
}
/**
- * Return volume info for the specified volume. If the volume does not
- * exist, returns {@code null}.
+ * Return volume info that updated usageBytes for the specified volume.
* @param omMetadataManager
* @param volume
+ * @param updateUsage
* @return OmVolumeArgs
* @throws IOException
*/
- protected OmVolumeArgs getVolumeInfo(OMMetadataManager omMetadataManager,
- String volume) {
-
-OmVolumeArgs volumeArgs = null;
-
-CacheValue value =
-omMetadataManager.getVolumeTable().getCacheValue(
-new CacheKey<>(omMetadataManager.getVolumeKey(volume)));
-
-if (value != null) {
- volumeArgs = value.getCacheValue();
-}
+ protected static synchronized OmVolumeArgs syncUpdateUsage(
+ OMMetadataManager omMetadataManager, String volume, long updateUsage) {
+OmVolumeArgs volumeArgs = omMetadataManager.getVolumeTable().getCacheValue(
+new CacheKey<>(omMetadataManager.getVolumeKey(volume)))
+.getCacheValue();
+volumeArgs.getUsedBytes().add(updateUsage);
+return volumeArgs.copyObject();
+ }
-return volumeArgs;
+ /**
+ * Return volume info that updated usageBytes for the specified volume. And
+ * check Volume usageBytes quota.
+ * @param omMetadataManager
+ * @param volume
+ * @param updateUsage
+ * @return OmVolumeArgs
+ * @throws IOException
+ */
+ protected static synchronized OmVolumeArgs syncCheckAndUpdateUsage(
Review comment:
> I have not understood one part, how without volume lock this will help
here.
>
> Because other threads can be updating volumeArgs when this update is
happening/ other threads read omVolumeArgs.
This is a possible case. Can lead to update usageBytes inaccurate.
Suppose that ThreadA update VolumeArgs using OMVolumeSetOwnerRequest, it
gets VolumeArgs from DB if usageBytes is 1000 at this point. When ThreadB
writes the new key using OMKeyCreateRequest, he changes usageBytes to 1100.
There is no Volume lock at this point, so ThreadB may finish executing first
and ThreadA last. The Final usageBytes for VolumeArgs would be 1000. This
should be a problem.
Maybe we still can't avoid using Volume lock. Any suggestions here?
@linyiqun
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] bharatviswa504 commented on a change in pull request #1489: HDDS-4308. Fix issue with quota update
bharatviswa504 commented on a change in pull request #1489:
URL: https://github.com/apache/ozone/pull/1489#discussion_r513169793
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##
@@ -597,27 +596,40 @@ protected boolean checkDirectoryAlreadyExists(String
volumeName,
}
/**
- * Return volume info for the specified volume. If the volume does not
- * exist, returns {@code null}.
+ * Return volume info that updated usageBytes for the specified volume.
* @param omMetadataManager
* @param volume
+ * @param updateUsage
* @return OmVolumeArgs
* @throws IOException
*/
- protected OmVolumeArgs getVolumeInfo(OMMetadataManager omMetadataManager,
- String volume) {
-
-OmVolumeArgs volumeArgs = null;
-
-CacheValue value =
-omMetadataManager.getVolumeTable().getCacheValue(
-new CacheKey<>(omMetadataManager.getVolumeKey(volume)));
-
-if (value != null) {
- volumeArgs = value.getCacheValue();
-}
+ protected static synchronized OmVolumeArgs syncUpdateUsage(
+ OMMetadataManager omMetadataManager, String volume, long updateUsage) {
+OmVolumeArgs volumeArgs = omMetadataManager.getVolumeTable().getCacheValue(
+new CacheKey<>(omMetadataManager.getVolumeKey(volume)))
+.getCacheValue();
+volumeArgs.getUsedBytes().add(updateUsage);
+return volumeArgs.copyObject();
+ }
-return volumeArgs;
+ /**
+ * Return volume info that updated usageBytes for the specified volume. And
+ * check Volume usageBytes quota.
+ * @param omMetadataManager
+ * @param volume
+ * @param updateUsage
+ * @return OmVolumeArgs
+ * @throws IOException
+ */
+ protected static synchronized OmVolumeArgs syncCheckAndUpdateUsage(
Review comment:
Do we need LongAdder still?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] bharatviswa504 commented on a change in pull request #1489: HDDS-4308. Fix issue with quota update
bharatviswa504 commented on a change in pull request #1489:
URL: https://github.com/apache/ozone/pull/1489#discussion_r513169229
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##
@@ -597,27 +596,40 @@ protected boolean checkDirectoryAlreadyExists(String
volumeName,
}
/**
- * Return volume info for the specified volume. If the volume does not
- * exist, returns {@code null}.
+ * Return volume info that updated usageBytes for the specified volume.
* @param omMetadataManager
* @param volume
+ * @param updateUsage
* @return OmVolumeArgs
* @throws IOException
*/
- protected OmVolumeArgs getVolumeInfo(OMMetadataManager omMetadataManager,
- String volume) {
-
-OmVolumeArgs volumeArgs = null;
-
-CacheValue value =
-omMetadataManager.getVolumeTable().getCacheValue(
-new CacheKey<>(omMetadataManager.getVolumeKey(volume)));
-
-if (value != null) {
- volumeArgs = value.getCacheValue();
-}
+ protected static synchronized OmVolumeArgs syncUpdateUsage(
+ OMMetadataManager omMetadataManager, String volume, long updateUsage) {
+OmVolumeArgs volumeArgs = omMetadataManager.getVolumeTable().getCacheValue(
+new CacheKey<>(omMetadataManager.getVolumeKey(volume)))
+.getCacheValue();
+volumeArgs.getUsedBytes().add(updateUsage);
+return volumeArgs.copyObject();
+ }
-return volumeArgs;
+ /**
+ * Return volume info that updated usageBytes for the specified volume. And
+ * check Volume usageBytes quota.
+ * @param omMetadataManager
+ * @param volume
+ * @param updateUsage
+ * @return OmVolumeArgs
+ * @throws IOException
+ */
+ protected static synchronized OmVolumeArgs syncCheckAndUpdateUsage(
Review comment:
I have not understood one part, how without volume lock this will help
here.
Because other threads can be updating volumeArgs when this update is
happening/ other threads read omVolumeArgs.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] bharatviswa504 commented on a change in pull request #1498: HDDS-4339. Allow AWSSignatureProcessor init when aws signature is absent.
bharatviswa504 commented on a change in pull request #1498:
URL: https://github.com/apache/ozone/pull/1498#discussion_r513166313
##
File path:
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneClientProducer.java
##
@@ -80,9 +81,15 @@ private OzoneClient getClient(OzoneConfiguration config)
throws IOException {
UserGroupInformation remoteUser =
Review comment:
This will throw NPE when awsAccessId is null, do we need to move
validateAccessId before remoteUser creation.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] captainzmc commented on pull request #1497: HDDS-4345. Replace the deprecated Lock method
captainzmc commented on pull request #1497: URL: https://github.com/apache/ozone/pull/1497#issuecomment-717654070 Thanks for @xiaoyuyao’s feedback. The issues has been 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: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [ozone] captainzmc commented on a change in pull request #1489: HDDS-4308. Fix issue with quota update
captainzmc commented on a change in pull request #1489:
URL: https://github.com/apache/ozone/pull/1489#discussion_r513140628
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##
@@ -597,27 +596,40 @@ protected boolean checkDirectoryAlreadyExists(String
volumeName,
}
/**
- * Return volume info for the specified volume. If the volume does not
- * exist, returns {@code null}.
+ * Return volume info that updated usageBytes for the specified volume.
* @param omMetadataManager
* @param volume
+ * @param updateUsage
* @return OmVolumeArgs
* @throws IOException
*/
- protected OmVolumeArgs getVolumeInfo(OMMetadataManager omMetadataManager,
- String volume) {
-
-OmVolumeArgs volumeArgs = null;
-
-CacheValue value =
-omMetadataManager.getVolumeTable().getCacheValue(
-new CacheKey<>(omMetadataManager.getVolumeKey(volume)));
-
-if (value != null) {
- volumeArgs = value.getCacheValue();
-}
+ protected static synchronized OmVolumeArgs syncUpdateUsage(
+ OMMetadataManager omMetadataManager, String volume, long updateUsage) {
+OmVolumeArgs volumeArgs = omMetadataManager.getVolumeTable().getCacheValue(
+new CacheKey<>(omMetadataManager.getVolumeKey(volume)))
+.getCacheValue();
+volumeArgs.getUsedBytes().add(updateUsage);
+return volumeArgs.copyObject();
+ }
-return volumeArgs;
+ /**
+ * Return volume info that updated usageBytes for the specified volume. And
+ * check Volume usageBytes quota.
+ * @param omMetadataManager
+ * @param volume
+ * @param updateUsage
+ * @return OmVolumeArgs
+ * @throws IOException
+ */
+ protected static synchronized OmVolumeArgs syncCheckAndUpdateUsage(
Review comment:
Thanks for @linyiqun's review.
Modifying getVolumeInfo to synchronized and get copyObject would not
suffice. There is only one instance of volumeArgs in memory, and we need to
update volumeArgs atomic after getVolumeInfo. Then get the value of the
copyObject.
So, I made a modification based on your suggestion, added update
volumeArgs's usedBytes to the getVolumeInfo method.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1515: HDDS-4373. [Design] Ozone support append operation
arp7 commented on a change in pull request #1515: URL: https://github.com/apache/hadoop-ozone/pull/1515#discussion_r513026161 ## File path: hadoop-hdds/docs/content/design/append.md ## @@ -0,0 +1,87 @@ +--- +title: Append +summary: Append to the existing key. +date: 2020-10-22 +jira: HDDS-4333 +status: implementing +author: captainzmc +--- + + +## Introduction +This is a proposal to introduce append operation for Ozone, which will allow write data in the tail of an existing file. + +## Goals + OzoneClient and OzoneFS Client support append operation. + While the original key is appended to the write, the key needs to be readable by other clients. + After the OutputStream of the new Append operation calls close, other clients can read the new Append content. This ensures consistency of read operations. +## Non-goals +The operation of hflush is not within the scope of this design. Created HDDS-4353 to discuss this. +## Related jira +https://issues.apache.org/jira/browse/HDDS-4333 +## Implementation +### Background conditions： +We can't currently open a closed Container. If append generates a new block every time, the key may have many smaller blocks less than 256MB(Default block size). Too many blocks will make the DB larger and also have an impact on read performance. + +### Solution： +When Append occurs, determine if the container for the last block is closed. If it's closed, we create a new block. if it's open we append data to the last block. This can avoid creating new blocks as much as possible. + +### Request process： + + + 1. Client executes append key operation to OM + + 2. OM checks if the key is in appendTable; if so, the key is being called by another client append. we cannot append this key at this point. If not, add the key to appendTable. + + 3. Check whether the last block of the key belongs to a closed container, if so, apply to SCM allocate a new block, if not, use the current block directly. Review comment: Blocks must be immutable, we should never modify the contents of a block. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #1515: HDDS-4373. [Design] Ozone support append operation
arp7 commented on a change in pull request #1515: URL: https://github.com/apache/hadoop-ozone/pull/1515#discussion_r513025813 ## File path: hadoop-hdds/docs/content/design/append.md ## @@ -0,0 +1,87 @@ +--- +title: Append +summary: Append to the existing key. +date: 2020-10-22 +jira: HDDS-4333 +status: implementing +author: captainzmc +--- + + +## Introduction +This is a proposal to introduce append operation for Ozone, which will allow write data in the tail of an existing file. + +## Goals + OzoneClient and OzoneFS Client support append operation. + While the original key is appended to the write, the key needs to be readable by other clients. + After the OutputStream of the new Append operation calls close, other clients can read the new Append content. This ensures consistency of read operations. +## Non-goals +The operation of hflush is not within the scope of this design. Created HDDS-4353 to discuss this. +## Related jira +https://issues.apache.org/jira/browse/HDDS-4333 +## Implementation +### Background conditions： +We can't currently open a closed Container. If append generates a new block every time, the key may have many smaller blocks less than 256MB(Default block size). Too many blocks will make the DB larger and also have an impact on read performance. + +### Solution： +When Append occurs, determine if the container for the last block is closed. If it's closed, we create a new block. if it's open we append data to the last block. This can avoid creating new blocks as much as possible. + +### Request process： + + + 1. Client executes append key operation to OM + + 2. OM checks if the key is in appendTable; if so, the key is being called by another client append. we cannot append this key at this point. If not, add the key to appendTable. Review comment: Why not have the last append win? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] prashantpogde commented on pull request #1507: HDDS-4307.Start Trash Emptier in Ozone Manager
prashantpogde commented on pull request #1507: URL: https://github.com/apache/hadoop-ozone/pull/1507#issuecomment-717415113 +1 LGTM This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao merged pull request #1516: HDDS-3731. [doc]add storage space quota document.
xiaoyuyao merged pull request #1516: URL: https://github.com/apache/hadoop-ozone/pull/1516 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on pull request #1516: HDDS-3731. [doc]add storage space quota document.
xiaoyuyao commented on pull request #1516: URL: https://github.com/apache/hadoop-ozone/pull/1516#issuecomment-717361668 Thanks @captainzmc for the update. LGTM, +1. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] linyiqun commented on a change in pull request #1489: HDDS-4308. Fix issue with quota update
linyiqun commented on a change in pull request #1489:
URL: https://github.com/apache/hadoop-ozone/pull/1489#discussion_r512749950
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##
@@ -597,27 +596,40 @@ protected boolean checkDirectoryAlreadyExists(String
volumeName,
}
/**
- * Return volume info for the specified volume. If the volume does not
- * exist, returns {@code null}.
+ * Return volume info that updated usageBytes for the specified volume.
* @param omMetadataManager
* @param volume
+ * @param updateUsage
* @return OmVolumeArgs
* @throws IOException
*/
- protected OmVolumeArgs getVolumeInfo(OMMetadataManager omMetadataManager,
- String volume) {
-
-OmVolumeArgs volumeArgs = null;
-
-CacheValue value =
-omMetadataManager.getVolumeTable().getCacheValue(
-new CacheKey<>(omMetadataManager.getVolumeKey(volume)));
-
-if (value != null) {
- volumeArgs = value.getCacheValue();
-}
+ protected static synchronized OmVolumeArgs syncUpdateUsage(
+ OMMetadataManager omMetadataManager, String volume, long updateUsage) {
+OmVolumeArgs volumeArgs = omMetadataManager.getVolumeTable().getCacheValue(
+new CacheKey<>(omMetadataManager.getVolumeKey(volume)))
+.getCacheValue();
+volumeArgs.getUsedBytes().add(updateUsage);
+return volumeArgs.copyObject();
+ }
-return volumeArgs;
+ /**
+ * Return volume info that updated usageBytes for the specified volume. And
+ * check Volume usageBytes quota.
+ * @param omMetadataManager
+ * @param volume
+ * @param updateUsage
+ * @return OmVolumeArgs
+ * @throws IOException
+ */
+ protected static synchronized OmVolumeArgs syncCheckAndUpdateUsage(
Review comment:
I don't prefer to add new methods here, this makes current PR not clear
to understand.
@captainzmc , can you make the minor adjustment for getVolumeInfo as I
suggested in JIRA HDDS-4308. After this, we can make few lines change I think.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] linyiqun commented on a change in pull request #1489: HDDS-4308. Fix issue with quota update
linyiqun commented on a change in pull request #1489:
URL: https://github.com/apache/hadoop-ozone/pull/1489#discussion_r512749950
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##
@@ -597,27 +596,40 @@ protected boolean checkDirectoryAlreadyExists(String
volumeName,
}
/**
- * Return volume info for the specified volume. If the volume does not
- * exist, returns {@code null}.
+ * Return volume info that updated usageBytes for the specified volume.
* @param omMetadataManager
* @param volume
+ * @param updateUsage
* @return OmVolumeArgs
* @throws IOException
*/
- protected OmVolumeArgs getVolumeInfo(OMMetadataManager omMetadataManager,
- String volume) {
-
-OmVolumeArgs volumeArgs = null;
-
-CacheValue value =
-omMetadataManager.getVolumeTable().getCacheValue(
-new CacheKey<>(omMetadataManager.getVolumeKey(volume)));
-
-if (value != null) {
- volumeArgs = value.getCacheValue();
-}
+ protected static synchronized OmVolumeArgs syncUpdateUsage(
+ OMMetadataManager omMetadataManager, String volume, long updateUsage) {
+OmVolumeArgs volumeArgs = omMetadataManager.getVolumeTable().getCacheValue(
+new CacheKey<>(omMetadataManager.getVolumeKey(volume)))
+.getCacheValue();
+volumeArgs.getUsedBytes().add(updateUsage);
+return volumeArgs.copyObject();
+ }
-return volumeArgs;
+ /**
+ * Return volume info that updated usageBytes for the specified volume. And
+ * check Volume usageBytes quota.
+ * @param omMetadataManager
+ * @param volume
+ * @param updateUsage
+ * @return OmVolumeArgs
+ * @throws IOException
+ */
+ protected static synchronized OmVolumeArgs syncCheckAndUpdateUsage(
Review comment:
I don't prefer to add new methods here, this makes current PR not clear
to understand.
@captainzmc , can you make the minor adjustment for getVolumeInfo as I
suggested in JIRA HDDS-4308.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] bshashikant commented on pull request #1523: HDDS-4320. Let Ozone input streams implement CanUnbuffer
bshashikant commented on pull request #1523: URL: https://github.com/apache/hadoop-ozone/pull/1523#issuecomment-717257127 Thanks @adoroszlai . I am still reviewing this, however, i have couple of questions: 1) In unbuffer, do we need to remove the corresponding blockInputStreams and chunkInputStreams which are already read? 2) While the connection to datanode is closed, connection to OM still is kept open, do we need to close this as well? 3) As i see the last position is cached, so after unbuffer is called, it will go into corresponding blockInputStream and chunkInputStream and starts reading again. What if the pipelne is not valid anymore i.e, the datanodes containing the blocks is replicated to a different set of datanodes? Do we need to handle this? As far as i remember , we only refresh the pipeline during initialisation of blockInputStream only. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] avijayanhwx merged pull request #1518: HDDS-4386: Each EndpointStateMachine uses its own thread pool to talk with SCM/Recon
avijayanhwx merged pull request #1518: URL: https://github.com/apache/hadoop-ozone/pull/1518 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] sadanand48 commented on a change in pull request #1507: HDDS-4307.Start Trash Emptier in Ozone Manager
sadanand48 commented on a change in pull request #1507:
URL: https://github.com/apache/hadoop-ozone/pull/1507#discussion_r512567525
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##
@@ -1228,17 +1238,57 @@ public void restart() throws IOException {
// Allow OM to start as Http Server failure is not fatal.
LOG.error("OM HttpServer failed to start.", ex);
}
-
omRpcServer.start();
+
isOmRpcServerRunning = true;
+startTrashEmptier(configuration);
+
registerMXBean();
startJVMPauseMonitor();
setStartTime();
omState = State.RUNNING;
}
+
+ /**
+ * @param conf
+ * @throws IOException
+ * Starts a Trash Emptier thread that does an fs.trashRoots and performs
+ * checkpointing & deletion
+ */
+ private void startTrashEmptier(Configuration conf) throws IOException {
+long trashInterval =
+conf.getLong(FS_TRASH_INTERVAL_KEY, FS_TRASH_INTERVAL_DEFAULT);
+if (trashInterval == 0) {
+ return;
Review comment:
Done.
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##
@@ -1228,17 +1238,57 @@ public void restart() throws IOException {
// Allow OM to start as Http Server failure is not fatal.
LOG.error("OM HttpServer failed to start.", ex);
}
-
omRpcServer.start();
+
isOmRpcServerRunning = true;
+startTrashEmptier(configuration);
+
registerMXBean();
startJVMPauseMonitor();
setStartTime();
omState = State.RUNNING;
}
+
+ /**
+ * @param conf
+ * @throws IOException
+ * Starts a Trash Emptier thread that does an fs.trashRoots and performs
Review comment:
Done
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai opened a new pull request #1525: HDDS-3959. Avoid HddsProtos.PipelineID#toString
adoroszlai opened a new pull request #1525: URL: https://github.com/apache/hadoop-ozone/pull/1525 ## What changes were proposed in this pull request? Change `CreatePipelineCommandHandler` and `ClosePipelineCommandHandler` to use the non-proto `PipelineID` object for logging. This lets us avoid printing multi-line messages, mostly caused by `uuid128` structure. Also tweak logic a bit to avoid unnecessary back and forth proto conversion for getting the node list in pipeline creation. https://issues.apache.org/jira/browse/HDDS-3959 ## How was this patch tested? Ran `ozone` docker compose cluster, closed one pipeline manually, verified log messages: ``` datanode_1 | 2020-10-27 09:58:13,533 [Command processor thread] INFO commandhandler.CreatePipelineCommandHandler: Created Pipeline RATIS THREE PipelineID=658bfc91-c07c-405f-94ab-7b7a87c9dd2c. ... datanode_2 | 2020-10-27 10:02:00,686 [Command processor thread] INFO commandhandler.ClosePipelineCommandHandler: Close Pipeline PipelineID=658bfc91-c07c-405f-94ab-7b7a87c9dd2c command on datanode fea867c0-2e70-45ec-8491-be7f952d91a4. ``` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1489: HDDS-4308. Fix issue with quota update
captainzmc commented on pull request #1489: URL: https://github.com/apache/hadoop-ozone/pull/1489#issuecomment-717123932 Hi @linyiqun,I modified the implementation based on the latest comments. Can you help to review this PR. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request #1507: HDDS-4307.Start Trash Emptier in Ozone Manager
rakeshadr commented on a change in pull request #1507:
URL: https://github.com/apache/hadoop-ozone/pull/1507#discussion_r512525048
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##
@@ -1228,17 +1238,57 @@ public void restart() throws IOException {
// Allow OM to start as Http Server failure is not fatal.
LOG.error("OM HttpServer failed to start.", ex);
}
-
omRpcServer.start();
+
isOmRpcServerRunning = true;
+startTrashEmptier(configuration);
+
registerMXBean();
startJVMPauseMonitor();
setStartTime();
omState = State.RUNNING;
}
+
+ /**
+ * @param conf
+ * @throws IOException
+ * Starts a Trash Emptier thread that does an fs.trashRoots and performs
Review comment:
Please follow general guidelines for the javadoc.
1) Begins with function details.
2) Provide `@param` details.
3) Then `@return` info.
4) Ending with `@throws` exception cases.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request #1507: HDDS-4307.Start Trash Emptier in Ozone Manager
rakeshadr commented on a change in pull request #1507:
URL: https://github.com/apache/hadoop-ozone/pull/1507#discussion_r512525048
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##
@@ -1228,17 +1238,57 @@ public void restart() throws IOException {
// Allow OM to start as Http Server failure is not fatal.
LOG.error("OM HttpServer failed to start.", ex);
}
-
omRpcServer.start();
+
isOmRpcServerRunning = true;
+startTrashEmptier(configuration);
+
registerMXBean();
startJVMPauseMonitor();
setStartTime();
omState = State.RUNNING;
}
+
+ /**
+ * @param conf
+ * @throws IOException
+ * Starts a Trash Emptier thread that does an fs.trashRoots and performs
Review comment:
Please follow general guidelines for the javadoc.
1) Begins with function details.
2) Provide `@param` details.
3) Then `@throws` exception cases.
4) Ending with `@return` info.
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##
@@ -1228,17 +1238,57 @@ public void restart() throws IOException {
// Allow OM to start as Http Server failure is not fatal.
LOG.error("OM HttpServer failed to start.", ex);
}
-
omRpcServer.start();
+
isOmRpcServerRunning = true;
+startTrashEmptier(configuration);
+
registerMXBean();
startJVMPauseMonitor();
setStartTime();
omState = State.RUNNING;
}
+
+ /**
+ * @param conf
+ * @throws IOException
+ * Starts a Trash Emptier thread that does an fs.trashRoots and performs
+ * checkpointing & deletion
+ */
+ private void startTrashEmptier(Configuration conf) throws IOException {
+long trashInterval =
+conf.getLong(FS_TRASH_INTERVAL_KEY, FS_TRASH_INTERVAL_DEFAULT);
+if (trashInterval == 0) {
+ return;
Review comment:
Please add a warn or even a lighter info log message to make the
behavior loud to the users as this will disable trash emptier.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc edited a comment on pull request #1489: HDDS-4308. Fix issue with quota update
captainzmc edited a comment on pull request #1489: URL: https://github.com/apache/hadoop-ozone/pull/1489#issuecomment-708182129 Hi @bharatviswa504, Can you help to review this PR. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] lokeshj1703 commented on pull request #1519: HDDS-4388. Make writeStateMachineTimeout retry count proportional to node failure timeout
lokeshj1703 commented on pull request #1519: URL: https://github.com/apache/hadoop-ozone/pull/1519#issuecomment-717039024 @bshashikant Thanks for the contribution! I have merged the PR to master branch. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] lokeshj1703 closed pull request #1519: HDDS-4388. Make writeStateMachineTimeout retry count proportional to node failure timeout
lokeshj1703 closed pull request #1519: URL: https://github.com/apache/hadoop-ozone/pull/1519 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on pull request #1503: HDDS-4332: ListFileStatus - do lookup in directory and file tables
rakeshadr commented on pull request #1503: URL: https://github.com/apache/hadoop-ozone/pull/1503#issuecomment-717036194 > Thanks for updating the PR, @rakeshadr . One further review comment below. > > In additional, current test change not fully cover the test for listStatusV1. Example, current OzoneFS test change doesn't address the case for listStatus with other startKey specified. > I see the test class TestKeyManagerImpl does the good coverage for listStatus call, can we add a test unit like that or make a minor refactor based on that? Thanks @linyiqun . Thats really a good point. I also looked at keyManagerImpl UT and it requires some refactoring effort to modify the createDir and other logic. I will work on and update you. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request #1503: HDDS-4332: ListFileStatus - do lookup in directory and file tables
rakeshadr commented on a change in pull request #1503:
URL: https://github.com/apache/hadoop-ozone/pull/1503#discussion_r512452877
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##
@@ -2205,6 +2276,318 @@ private void listStatusFindKeyInTableCache(
return fileStatusList;
}
+ public List listStatusV1(OmKeyArgs args, boolean recursive,
+ String startKey, long numEntries, String clientAddress)
+ throws IOException {
+Preconditions.checkNotNull(args, "Key args can not be null");
+
+// unsorted OMKeyInfo list contains combine results from TableCache and DB.
+List fileStatusFinalList = new ArrayList<>();
+LinkedHashSet fileStatusList = new LinkedHashSet<>();
+if (numEntries <= 0) {
+ return fileStatusFinalList;
+}
+
+String volumeName = args.getVolumeName();
+String bucketName = args.getBucketName();
+String keyName = args.getKeyName();
+String seekFileInDB;
+String seekDirInDB;
+long prefixKeyInDB;
+String prefixPath = keyName;
+
+int countEntries = 0;
+
+metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName,
+bucketName);
+try {
+ if (Strings.isNullOrEmpty(startKey)) {
+OzoneFileStatus fileStatus = getFileStatus(args, clientAddress);
+if (fileStatus.isFile()) {
+ return Collections.singletonList(fileStatus);
+}
+
+// Not required to search in DeletedTable because all the deleted
+// keys will be marked directly in dirTable or in keyTable by
+// breaking the pointer to its sub-dirs. So, there is no issue of
+// inconsistency.
+
+/*
+ * keyName is a directory.
+ * Say, "/a" is the dir name and its objectID is 1024, then seek
+ * will be doing with "1024/" to get all immediate descendants.
+ */
+if (fileStatus.getKeyInfo() != null) {
+ prefixKeyInDB = fileStatus.getKeyInfo().getObjectID();
+} else {
+ // list root directory.
+ String bucketKey = metadataManager.getBucketKey(volumeName,
+ bucketName);
+ OmBucketInfo omBucketInfo =
+ metadataManager.getBucketTable().get(bucketKey);
+ prefixKeyInDB = omBucketInfo.getObjectID();
+}
+seekFileInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
+seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
+
+// TODO: recursive flag=true will be handled in HDDS-4360 jira.
+// Order of seek -> (1)Seek dirs in dirTable (2)Seek files in fileTable
+// 1. Seek the given key in key table.
+countEntries = getFilesFromDirectory(fileStatusList, seekFileInDB,
+prefixPath, prefixKeyInDB, startKey, countEntries, numEntries);
+// 2. Seek the given key in dir table.
+getDirectories(recursive, startKey, numEntries, fileStatusList,
+volumeName, bucketName, seekDirInDB, prefixKeyInDB,
+prefixPath, countEntries);
+ } else {
+/*
+ * startKey will be used in iterator seek and sets the beginning point
+ * for key traversal.
+ *
+ * key name will be used as parentID where the user has requested to
+ * list the keys from.
+ *
+ * When recursive flag=false, parentID won't change between two pages.
+ * For example: OM has a namespace like,
+ */a/1...1M files and /a/b/1...1M files.
+ */a/1...1M directories and /a/b/1...1M directories.
+ * Listing "/a", will always have the parentID as "a" irrespective of
+ * the startKey value.
+ */
+// TODO: recursive flag=true will be handled in HDDS-4360 jira.
+OzoneFileStatus fileStatusInfo = getOzoneFileStatusV1(volumeName,
+bucketName, startKey, false, null, true);
+
+if (fileStatusInfo != null) {
+ prefixKeyInDB = fileStatusInfo.getKeyInfo().getParentObjectID();
+ if(fileStatusInfo.isDirectory()){
+seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB,
+fileStatusInfo.getKeyInfo().getFileName());
+
+// Order of seek -> (1) Seek dirs in dirTable. In OM, always the
+// order of search is, first seek into fileTable and then dirTable.
+// So, its not required to search again into the fileTable.
+
+// Seek the given key in dirTable.
+getDirectories(recursive, startKey, numEntries,
+fileStatusList, volumeName, bucketName, seekDirInDB,
+prefixKeyInDB, prefixPath, countEntries);
+
+ } else {
+seekFileInDB = metadataManager.getOzonePathKey(prefixKeyInDB,
Review comment:
As we have to perform seek in two tables to finish the listing, I'm
maintaining a seek order
1) Seek all the files from
[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request #1503: HDDS-4332: ListFileStatus - do lookup in directory and file tables
rakeshadr commented on a change in pull request #1503:
URL: https://github.com/apache/hadoop-ozone/pull/1503#discussion_r512452877
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##
@@ -2205,6 +2276,318 @@ private void listStatusFindKeyInTableCache(
return fileStatusList;
}
+ public List listStatusV1(OmKeyArgs args, boolean recursive,
+ String startKey, long numEntries, String clientAddress)
+ throws IOException {
+Preconditions.checkNotNull(args, "Key args can not be null");
+
+// unsorted OMKeyInfo list contains combine results from TableCache and DB.
+List fileStatusFinalList = new ArrayList<>();
+LinkedHashSet fileStatusList = new LinkedHashSet<>();
+if (numEntries <= 0) {
+ return fileStatusFinalList;
+}
+
+String volumeName = args.getVolumeName();
+String bucketName = args.getBucketName();
+String keyName = args.getKeyName();
+String seekFileInDB;
+String seekDirInDB;
+long prefixKeyInDB;
+String prefixPath = keyName;
+
+int countEntries = 0;
+
+metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName,
+bucketName);
+try {
+ if (Strings.isNullOrEmpty(startKey)) {
+OzoneFileStatus fileStatus = getFileStatus(args, clientAddress);
+if (fileStatus.isFile()) {
+ return Collections.singletonList(fileStatus);
+}
+
+// Not required to search in DeletedTable because all the deleted
+// keys will be marked directly in dirTable or in keyTable by
+// breaking the pointer to its sub-dirs. So, there is no issue of
+// inconsistency.
+
+/*
+ * keyName is a directory.
+ * Say, "/a" is the dir name and its objectID is 1024, then seek
+ * will be doing with "1024/" to get all immediate descendants.
+ */
+if (fileStatus.getKeyInfo() != null) {
+ prefixKeyInDB = fileStatus.getKeyInfo().getObjectID();
+} else {
+ // list root directory.
+ String bucketKey = metadataManager.getBucketKey(volumeName,
+ bucketName);
+ OmBucketInfo omBucketInfo =
+ metadataManager.getBucketTable().get(bucketKey);
+ prefixKeyInDB = omBucketInfo.getObjectID();
+}
+seekFileInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
+seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
+
+// TODO: recursive flag=true will be handled in HDDS-4360 jira.
+// Order of seek -> (1)Seek dirs in dirTable (2)Seek files in fileTable
+// 1. Seek the given key in key table.
+countEntries = getFilesFromDirectory(fileStatusList, seekFileInDB,
+prefixPath, prefixKeyInDB, startKey, countEntries, numEntries);
+// 2. Seek the given key in dir table.
+getDirectories(recursive, startKey, numEntries, fileStatusList,
+volumeName, bucketName, seekDirInDB, prefixKeyInDB,
+prefixPath, countEntries);
+ } else {
+/*
+ * startKey will be used in iterator seek and sets the beginning point
+ * for key traversal.
+ *
+ * key name will be used as parentID where the user has requested to
+ * list the keys from.
+ *
+ * When recursive flag=false, parentID won't change between two pages.
+ * For example: OM has a namespace like,
+ */a/1...1M files and /a/b/1...1M files.
+ */a/1...1M directories and /a/b/1...1M directories.
+ * Listing "/a", will always have the parentID as "a" irrespective of
+ * the startKey value.
+ */
+// TODO: recursive flag=true will be handled in HDDS-4360 jira.
+OzoneFileStatus fileStatusInfo = getOzoneFileStatusV1(volumeName,
+bucketName, startKey, false, null, true);
+
+if (fileStatusInfo != null) {
+ prefixKeyInDB = fileStatusInfo.getKeyInfo().getParentObjectID();
+ if(fileStatusInfo.isDirectory()){
+seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB,
+fileStatusInfo.getKeyInfo().getFileName());
+
+// Order of seek -> (1) Seek dirs in dirTable. In OM, always the
+// order of search is, first seek into fileTable and then dirTable.
+// So, its not required to search again into the fileTable.
+
+// Seek the given key in dirTable.
+getDirectories(recursive, startKey, numEntries,
+fileStatusList, volumeName, bucketName, seekDirInDB,
+prefixKeyInDB, prefixPath, countEntries);
+
+ } else {
+seekFileInDB = metadataManager.getOzonePathKey(prefixKeyInDB,
Review comment:
As we have to perform seek in two tables to finish the listing, I'm
maintaining a seek order
1) Finish listing all the files
[GitHub] [hadoop-ozone] frischHWC opened a new pull request #1524: HDDS-4258.Set GDPR to a Security submenu in EN and CN document
frischHWC opened a new pull request #1524: URL: https://github.com/apache/hadoop-ozone/pull/1524 ## What changes were proposed in this pull request? Setting GDPR to security submenu for EN & CN pages. ## What is the link to the Apache JIRA https://issues.apache.org/jira/browse/HDDS-4258 ## How was this patch tested? Locally with 'hugo serve', see attached screenshot: https://user-images.githubusercontent.com/47358141/97264554-7aead900-1825-11eb-9a6e-01e940783511.png;> This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] sadanand48 commented on pull request #1507: HDDS-4307.Start Trash Emptier in Ozone Manager
sadanand48 commented on pull request #1507: URL: https://github.com/apache/hadoop-ozone/pull/1507#issuecomment-717002589 Thanks @rakeshadr for the review. Addressed your comments . Will incorporate the changes you suggested on the BackgroundService when i include it in the next patch. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] sadanand48 edited a comment on pull request #1507: HDDS-4307.Start Trash Emptier in Ozone Manager
sadanand48 edited a comment on pull request #1507: URL: https://github.com/apache/hadoop-ozone/pull/1507#issuecomment-717002589 Thanks @rakeshadr for the review. Addressed your comments . Will incorporate the changes you suggested on the TrashDeletingService when i include it in the next patch. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] GlenGeng commented on pull request #1518: HDDS-4386: Each EndpointStateMachine uses its own thread pool to talk with SCM/Recon
GlenGeng commented on pull request #1518: URL: https://github.com/apache/hadoop-ozone/pull/1518#issuecomment-716941212 @avijayanhwx We've create two Jiras for the Recon issue, HDDS-4385 and HDDS-4355. I've cc you in them. BTW, in our production cluster, since starting up recon will make the cluster dead, we can not debug the issue until next release with this fix. I guess we might have to wait for a while QAQ. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] avijayanhwx commented on pull request #1518: HDDS-4386: Each EndpointStateMachine uses its own thread pool to talk with SCM/Recon
avijayanhwx commented on pull request #1518: URL: https://github.com/apache/hadoop-ozone/pull/1518#issuecomment-716920861 > @GlenGeng Thanks for fixing this issue. > > Is there a JIRA created for the Recon slowness issue? > > Also, irrespective of whether there is a slow SCM or Recon, the DN should not submit a request to an endpoint if there is a pending request to that endpoint. Is that not true currently? Even if there are 2 threads in the same thread pool (1 for SCM and 1 for Recon), then a slow Recon should not affect SCM reports. Thanks for the explanation @GlenGeng. IMO, it is good to have Recon and SCM use different thread pools. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] github-actions[bot] commented on pull request #1327: HDDS-4098. Improve om admin getserviceroles error message
github-actions[bot] commented on pull request #1327: URL: https://github.com/apache/hadoop-ozone/pull/1327#issuecomment-716905708 Thank you very much for the patch. I am closing this PR __temporarily__ as there was no activity recently and it is waiting for response from its author. It doesn't mean that this PR is not important or ignored: feel free to reopen the PR at any time. It only means that attention of committers is not required. We prefer to keep the review queue clean. This ensures PRs in need of review are more visible, which results in faster feedback for all PRs. If you need ANY help to finish this PR, please [contact the community](https://github.com/apache/hadoop-ozone#contact) on the mailing list or the slack channel." This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] github-actions[bot] commented on pull request #1363: HDDS-3805. [OFS] Remove usage of OzoneClientAdapter interface
github-actions[bot] commented on pull request #1363: URL: https://github.com/apache/hadoop-ozone/pull/1363#issuecomment-716905696 Thank you very much for the patch. I am closing this PR __temporarily__ as there was no activity recently and it is waiting for response from its author. It doesn't mean that this PR is not important or ignored: feel free to reopen the PR at any time. It only means that attention of committers is not required. We prefer to keep the review queue clean. This ensures PRs in need of review are more visible, which results in faster feedback for all PRs. If you need ANY help to finish this PR, please [contact the community](https://github.com/apache/hadoop-ozone#contact) on the mailing list or the slack channel." This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] github-actions[bot] closed pull request #1327: HDDS-4098. Improve om admin getserviceroles error message
github-actions[bot] closed pull request #1327: URL: https://github.com/apache/hadoop-ozone/pull/1327 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] github-actions[bot] closed pull request #1363: HDDS-3805. [OFS] Remove usage of OzoneClientAdapter interface
github-actions[bot] closed pull request #1363: URL: https://github.com/apache/hadoop-ozone/pull/1363 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao merged pull request #1301: HDDS-3882. Update modification time when updating volume/bucket/key ACLs
xiaoyuyao merged pull request #1301: URL: https://github.com/apache/hadoop-ozone/pull/1301 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on pull request #1301: HDDS-3882. Update modification time when updating volume/bucket/key ACLs
xiaoyuyao commented on pull request #1301: URL: https://github.com/apache/hadoop-ozone/pull/1301#issuecomment-716788128 Thanks @cxorm for fixing this. The change LGTM, +1. I will merge it shortly. Also notice similar unnecessary builder conversion issue in OMVolumeSetQuotaRequestwhen the modify time is updated. Can you check and open separate JIRA to fix those? 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: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1497: HDDS-4345. Replace the deprecated Lock method
xiaoyuyao commented on a change in pull request #1497:
URL: https://github.com/apache/hadoop-ozone/pull/1497#discussion_r512223510
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/VolumeManagerImpl.java
##
@@ -620,7 +620,7 @@ public boolean checkAccess(OzoneObj ozObject,
RequestContext context)
Objects.requireNonNull(context);
String volume = ozObject.getVolumeName();
-metadataManager.getLock().acquireLock(VOLUME_LOCK, volume);
+metadataManager.getLock().acquireWriteLock(VOLUME_LOCK, volume);
Review comment:
This should be a ReadLock
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/VolumeManagerImpl.java
##
@@ -647,7 +647,7 @@ public boolean checkAccess(OzoneObj ozObject,
RequestContext context)
throw new OMException("Check access operation failed for " +
"volume:" + volume, ex, ResultCodes.INTERNAL_ERROR);
} finally {
- metadataManager.getLock().releaseLock(VOLUME_LOCK, volume);
+ metadataManager.getLock().releaseWriteLock(VOLUME_LOCK, volume);
Review comment:
This should be a ReadLock
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] prashantpogde commented on a change in pull request #1486: HDDS-4296. SCM changes to process Layout Info in heartbeat request/response
prashantpogde commented on a change in pull request #1486:
URL: https://github.com/apache/hadoop-ozone/pull/1486#discussion_r512168941
##
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
##
@@ -400,6 +405,66 @@ public void processNodeReport(DatanodeDetails
datanodeDetails,
}
}
+ /**
+ * Process Layout Version report.
+ *
+ * @param datanodeDetails
+ * @param layoutVersionReport
+ */
+ @Override
+ public void processLayoutVersionReport(DatanodeDetails datanodeDetails,
+LayoutVersionProto layoutVersionReport) {
+if (LOG.isDebugEnabled()) {
+ LOG.debug("Processing Layout Version report from [datanode={}]",
+ datanodeDetails.getHostName());
+}
+if (LOG.isTraceEnabled()) {
+ LOG.trace("HB is received from [datanode={}]: {}",
+ datanodeDetails.getHostName(),
+ layoutVersionReport.toString().replaceAll("\n", "n"));
+}
+
+if (layoutVersionReport != null) {
+ int scmSlv = scmLayoutVersionManager.getSoftwareLayoutVersion();
+ int scmMlv = scmLayoutVersionManager.getMetadataLayoutVersion();
+ UpgradeFinalizer.Status scmUpgradeState =
+ scmLayoutVersionManager.getUpgradeState();
+ int dnSlv = layoutVersionReport.getSoftwareLayoutVersion();
+ int dnMlv = layoutVersionReport.getMetadataLayoutVersion();
+
+ // If the data node slv is > scm slv => log error condition
+ if (dnSlv > scmSlv) {
Review comment:
yup
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao merged pull request #1395: HDDS-4088. Adding Owner info for Authorizer plugin to honor owner access rights
xiaoyuyao merged pull request #1395: URL: https://github.com/apache/hadoop-ozone/pull/1395 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on pull request #1395: HDDS-4088. Adding Owner info for Authorizer plugin to honor owner access rights
xiaoyuyao commented on pull request #1395: URL: https://github.com/apache/hadoop-ozone/pull/1395#issuecomment-716722366 Thanks @smengcl for the 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: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai opened a new pull request #1523: HDDS-4320. Let Ozone input streams implement CanUnbuffer
adoroszlai opened a new pull request #1523: URL: https://github.com/apache/hadoop-ozone/pull/1523 ## What changes were proposed in this pull request? Implement [`CanUnbuffer` interface](https://github.com/apache/hadoop/blob/b32926f1108bef1f1e506de684c021203b2432f1/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CanUnbuffer.java#L32-L35) in Ozone input streams (`ChunkInputStream`, `BlockInputStream`): release buffers and disconnect the client when `unbuffer()` is called. https://issues.apache.org/jira/browse/HDDS-4320 ## How was this patch tested? Added contract test (base test copied from Hadoop 3.3) for both OFS and O3FS: https://github.com/adoroszlai/hadoop-ozone/runs/1309754540#step:4:3093 https://github.com/adoroszlai/hadoop-ozone/runs/1309754540#step:4:3107 also added unit test: https://github.com/adoroszlai/hadoop-ozone/runs/1309754295#step:3:3540 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] asfgit merged pull request #1520: Merge master branch into HDDS-1880-Decom
asfgit merged pull request #1520: URL: https://github.com/apache/hadoop-ozone/pull/1520 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] codecov-io edited a comment on pull request #1520: Merge master branch into HDDS-1880-Decom
codecov-io edited a comment on pull request #1520: URL: https://github.com/apache/hadoop-ozone/pull/1520#issuecomment-715660815 # [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1520?src=pr=h1) Report > :exclamation: No coverage uploaded for pull request base (`HDDS-1880-Decom@f64476c`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit). > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/hadoop-ozone/pull/1520?src=pr=tree) ```diff @@Coverage Diff @@ ## HDDS-1880-Decom#1520 +/- ## == Coverage ? 75.41% Complexity ?10851 == Files ? 1030 Lines ?52339 Branches ? 5127 == Hits ?39473 Misses ?10420 Partials ? 2446 ``` -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1520?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1520?src=pr=footer). Last update [f64476c...18ed003](https://codecov.io/gh/apache/hadoop-ozone/pull/1520?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1521: HDDS-4362. Change hadoop32 test to use 3.2 image
adoroszlai commented on pull request #1521: URL: https://github.com/apache/hadoop-ozone/pull/1521#issuecomment-716548122 Thanks @elek for reviewing and committing it. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] elek merged pull request #1521: HDDS-4362. Change hadoop32 test to use 3.2 image
elek merged pull request #1521: URL: https://github.com/apache/hadoop-ozone/pull/1521 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] timmylicheng opened a new pull request #1522: Hdds 4393 Test CI failures after force push
timmylicheng opened a new pull request #1522: URL: https://github.com/apache/hadoop-ozone/pull/1522 ## What changes were proposed in this pull request? Test CI failures after force push (Please fill in changes proposed in this fix) ## What is the link to the Apache JIRA https://issues.apache.org/jira/browse/HDDS-4393 (Please create an issue in ASF JIRA before opening a pull request, and you need to set the title of the pull request which starts with the corresponding JIRA issue number. (e.g. HDDS-. Fix a typo in YYY.) Please replace this section with the link to the Apache JIRA) ## How was this patch tested? (Please explain how this patch was tested. Ex: unit tests, manual tests) (If this patch involves UI changes, please attach a screen-shot; otherwise, remove this) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1497: HDDS-4345. Replace the deprecated Lock method
captainzmc commented on pull request #1497: URL: https://github.com/apache/hadoop-ozone/pull/1497#issuecomment-716470302 Thanks for @xiaoyuyao‘s review, the issues has been 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: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] timmylicheng commented on pull request #1498: HDDS-4339. Allow AWSSignatureProcessor init when aws signature is absent.
timmylicheng commented on pull request #1498: URL: https://github.com/apache/hadoop-ozone/pull/1498#issuecomment-716269366 This fix is to keep AWSSignatureProcessor out of NPE so that when OzoneClientProducer could be instantiated even when auth is missing in header. After that OzoneClientProducer can return proper s3 auth errors as part of response, which we would track in HDDS-4361. @ChenSammi @bharatviswa504 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] cxorm commented on pull request #1301: HDDS-3882. Update modification time when updating volume/bucket/key ACLs
cxorm commented on pull request #1301: URL: https://github.com/apache/hadoop-ozone/pull/1301#issuecomment-716102947 > Thanks for the update @cxorm. A few similar usage that can be optimized. Sorry I did not explicitly comment all of them last time. That's my carelessness : ) Thanks a lot @xiaoyuyao for reviewing it again, the toBuilder issues in AclRequest has been addressed. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] codecov-io edited a comment on pull request #1520: Merge master branch into HDDS-1880-Decom
codecov-io edited a comment on pull request #1520: URL: https://github.com/apache/hadoop-ozone/pull/1520#issuecomment-715660815 # [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1520?src=pr=h1) Report > :exclamation: No coverage uploaded for pull request base (`HDDS-1880-Decom@f64476c`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit). > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/hadoop-ozone/pull/1520?src=pr=tree) ```diff @@Coverage Diff @@ ## HDDS-1880-Decom#1520 +/- ## == Coverage ? 75.41% Complexity ?10850 == Files ? 1030 Lines ?52339 Branches ? 5127 == Hits ?39472 Misses ?10420 Partials ? 2447 ``` -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1520?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1520?src=pr=footer). Last update [f64476c...18ed003](https://codecov.io/gh/apache/hadoop-ozone/pull/1520?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] cxorm commented on a change in pull request #1301: HDDS-3882. Update modification time when updating volume/bucket/key ACLs
cxorm commented on a change in pull request #1301: URL: https://github.com/apache/hadoop-ozone/pull/1301#discussion_r511484216 ## File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java ## @@ -84,6 +84,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, VOLUME_LOCK, volume); omVolumeArgs = getVolumeInfo(omMetadataManager, volume); + Review comment: nit: redundant line This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] cxorm commented on a change in pull request #1301: HDDS-3882. Update modification time when updating volume/bucket/key ACLs
cxorm commented on a change in pull request #1301: URL: https://github.com/apache/hadoop-ozone/pull/1301#discussion_r511484216 ## File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java ## @@ -84,6 +84,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, VOLUME_LOCK, volume); omVolumeArgs = getVolumeInfo(omMetadataManager, volume); + Review comment: nit This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai opened a new pull request #1521: HDDS-4362. Change hadoop32 test to use 3.2 image
adoroszlai opened a new pull request #1521: URL: https://github.com/apache/hadoop-ozone/pull/1521 ## What changes were proposed in this pull request? `ozone-mr/hadoop32` and `ozonesecure-mr` acceptance tests use "latest" `hadoop:3` docker image, which is currently Hadoop 3.2. If it gets updated to Hadoop 3.3, Ozone acceptance test will be broken. This PR changes to a specific 3.2-based image, similar to how `ozone-mr/hadoop31` uses 3.1-based image. https://issues.apache.org/jira/browse/HDDS-4362 ## How was this patch tested? https://github.com/adoroszlai/hadoop-ozone/runs/1294092363 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] codecov-io commented on pull request #1520: Merge master branch into HDDS-1880-Decom
codecov-io commented on pull request #1520: URL: https://github.com/apache/hadoop-ozone/pull/1520#issuecomment-715660815 # [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1520?src=pr=h1) Report > :exclamation: No coverage uploaded for pull request base (`HDDS-1880-Decom@f64476c`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit). > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/hadoop-ozone/pull/1520?src=pr=tree) ```diff @@Coverage Diff @@ ## HDDS-1880-Decom#1520 +/- ## == Coverage ? 75.40% Complexity ?10847 == Files ? 1030 Lines ?52339 Branches ? 5127 == Hits ?39466 Misses ?10424 Partials ? 2449 ``` -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1520?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1520?src=pr=footer). Last update [f64476c...18ed003](https://codecov.io/gh/apache/hadoop-ozone/pull/1520?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] GlenGeng commented on pull request #1518: HDDS-4386: Each EndpointStateMachine uses its own thread pool to talk with SCM/Recon
GlenGeng commented on pull request #1518:
URL: https://github.com/apache/hadoop-ozone/pull/1518#issuecomment-715656741
@avijayanhwx Thanks for looking at this PR.
@ChenSammi Could you please create a Jira for slow Recon ?
> Also, irrespective of whether there is a slow SCM or Recon, the DN should
not submit a request to an endpoint if there is a pending request to that
endpoint. Is that not true currently?
It is not true.
`DatanodeStateMachine.stateMachineThread` calls
`context.execute(executorService, heartbeatFrequency, TimeUnit.MILLISECONDS);`
every heartbeatFrequency.
The job is
```
DatanodeState task = getTask();
task.execute(service);
DatanodeStateMachine.DatanodeStates newState = task.await(time, unit);
```
where the task is either `InitDatanodeState` or `RunningDatanodeState`,
depending on state of Datanode state machine:
```
public DatanodeState getTask() {
switch (this.state) {
case INIT:
return new InitDatanodeState(this.conf, parent.getConnectionManager(),
this);
case RUNNING:
return new RunningDatanodeState(this.conf,
parent.getConnectionManager(),
this);
case SHUTDOWN:
return null;
default:
throw new IllegalArgumentException("Not Implemented yet.");
}
}
```
Say it is `RunningDatanodeState`, it will do a endpointTask for each
endpoint, which does not consider whether there is an outstanding request for
that endpoint.
```
public void execute(ExecutorService executor) {
ecs = new ExecutorCompletionService<>(executor);
for (EndpointStateMachine endpoint : connectionManager.getValues()) {
Callable endpointTask = getEndPointTask(endpoint);
if (endpointTask != null) {
ecs.submit(endpointTask);
} else {
// This can happen if a task is taking more time than the timeOut
// specified for the task in await, and when it is completed the task
// has set the state to Shutdown, we may see the state as shutdown
// here. So, we need to Shutdown DatanodeStateMachine.
LOG.error("State is Shutdown in RunningDatanodeState");
context.setState(DatanodeStateMachine.DatanodeStates.SHUTDOWN);
}
}
}
```
and the 'await()' will do a timely wait, so that it will not block the next
heartbeat task of DatanodeStateMachine
```
public DatanodeStateMachine.DatanodeStates
await(long duration, TimeUnit timeUnit)
throws InterruptedException {
while (returned < count && timeLeft > 0) {
Future result =
ecs.poll(timeLeft, TimeUnit.MILLISECONDS);
if (result != null) {
results.add(result);
returned++;
}
timeLeft = durationMS - (Time.monotonicNow() - startTime);
}
return computeNextContainerState(results);
}
```
Consider `HeartbeatEndpointTask`, its `call()` just acquire the lock and do
a heartbeat RPC:
```
public EndpointStateMachine.EndPointStates call() throws Exception {
rpcEndpoint.lock();
try {
...
SCMHeartbeatResponseProto reponse = rpcEndpoint.getEndPoint()
.sendHeartbeat(request);
processResponse(reponse, datanodeDetailsProto);
...
} catch (IOException ex) {
...
} finally {
rpcEndpoint.unlock();
}
return rpcEndpoint.getState();
}
```
Here is the problem:
Say there is a slow Recon, two threads in thread pool. During the first
heartbeat, one thread will acquire the lock and pending on the RPC with Recon,
during the next heartbeat, the idle thread, after it finishes the job with SCM,
it will finally blocked on acquiring endpoint lock of Recon, during the third
heartbeat, there will no available threads in the thread pool.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1497: HDDS-4345. Replace the deprecated Lock method
xiaoyuyao commented on a change in pull request #1497:
URL: https://github.com/apache/hadoop-ozone/pull/1497#discussion_r511191225
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/PrefixManagerImpl.java
##
@@ -253,18 +253,18 @@ public boolean checkAccess(OzoneObj ozObject,
RequestContext context)
return true;
}
} finally {
- metadataManager.getLock().releaseLock(PREFIX_LOCK, prefixPath);
+ metadataManager.getLock().releaseWriteLock(PREFIX_LOCK, prefixPath);
}
}
@Override
public List getLongestPrefixPath(String path) {
String prefixPath = prefixTree.getLongestPrefix(path);
-metadataManager.getLock().acquireLock(PREFIX_LOCK, prefixPath);
+metadataManager.getLock().acquireWriteLock(PREFIX_LOCK, prefixPath);
Review comment:
Same as above.
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/PrefixManagerImpl.java
##
@@ -232,7 +232,7 @@ public boolean checkAccess(OzoneObj ozObject,
RequestContext context)
Objects.requireNonNull(context);
String prefixPath = ozObject.getPath();
-metadataManager.getLock().acquireLock(PREFIX_LOCK, prefixPath);
+metadataManager.getLock().acquireWriteLock(PREFIX_LOCK, prefixPath);
Review comment:
Should we use acquireReadLock here?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] avijayanhwx commented on pull request #1518: HDDS-4386: Each EndpointStateMachine uses its own thread pool to talk with SCM/Recon
avijayanhwx commented on pull request #1518: URL: https://github.com/apache/hadoop-ozone/pull/1518#issuecomment-715582860 @GlenGeng Thanks for fixing this issue. Is there a JIRA created for the Recon slowness issue? Also, irrespective of whether there is a slow SCM or Recon, the DN should not submit a request to an endpoint if there is a pending request to that endpoint. Is that not true currently? Even if there are 2 threads in the same thread pool (1 for SCM and 1 for Recon), then a slow Recon should not affect SCM reports. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] avijayanhwx commented on a change in pull request #1486: HDDS-4296. SCM changes to process Layout Info in heartbeat request/response
avijayanhwx commented on a change in pull request #1486:
URL: https://github.com/apache/hadoop-ozone/pull/1486#discussion_r511140146
##
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
##
@@ -400,6 +405,66 @@ public void processNodeReport(DatanodeDetails
datanodeDetails,
}
}
+ /**
+ * Process Layout Version report.
+ *
+ * @param datanodeDetails
+ * @param layoutVersionReport
+ */
+ @Override
+ public void processLayoutVersionReport(DatanodeDetails datanodeDetails,
+LayoutVersionProto layoutVersionReport) {
+if (LOG.isDebugEnabled()) {
+ LOG.debug("Processing Layout Version report from [datanode={}]",
+ datanodeDetails.getHostName());
+}
+if (LOG.isTraceEnabled()) {
+ LOG.trace("HB is received from [datanode={}]: {}",
+ datanodeDetails.getHostName(),
+ layoutVersionReport.toString().replaceAll("\n", "n"));
+}
+
+if (layoutVersionReport != null) {
+ int scmSlv = scmLayoutVersionManager.getSoftwareLayoutVersion();
+ int scmMlv = scmLayoutVersionManager.getMetadataLayoutVersion();
+ UpgradeFinalizer.Status scmUpgradeState =
+ scmLayoutVersionManager.getUpgradeState();
+ int dnSlv = layoutVersionReport.getSoftwareLayoutVersion();
+ int dnMlv = layoutVersionReport.getMetadataLayoutVersion();
+
+ // If the data node slv is > scm slv => log error condition
+ if (dnSlv > scmSlv) {
Review comment:
@prashantpogde End to End test cases make sense for integration or
acceptance tests. Is it possible to add a unit test just for the
SCMNodeManager.processLayoutVersion method? Since it has a few condition checks
(path flows), it may be a good candidate for a unit test based on mocks.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] sodonnel opened a new pull request #1520: Merge master branch into HDDS-1880-Decom
sodonnel opened a new pull request #1520: URL: https://github.com/apache/hadoop-ozone/pull/1520 ## What changes were proposed in this pull request? Merge master into the decommission branch. This was a clean merge with no conflicts at all. ## What is the link to the Apache JIRA No Jira. ## How was this patch tested? Existing tests This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] lokeshj1703 commented on a change in pull request #1519: HDDS-4388. Make writeStateMachineTimeout retry count proportional to node failure timeout
lokeshj1703 commented on a change in pull request #1519:
URL: https://github.com/apache/hadoop-ozone/pull/1519#discussion_r510844138
##
File path:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java
##
@@ -200,6 +200,9 @@ private RaftProperties newRaftProperties() {
TimeUnit timeUnit;
long duration;
+// set the node failure timeout
+setNodeFailureTimeout(properties);
Review comment:
`setNodeFailureTimeout` initializes a field. I think we can initialize
the field in the constructor itself and remove this function.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] lokeshj1703 commented on pull request #1513: HDDS-4366. SCM deletion service should delete configured number of blocks every interval.
lokeshj1703 commented on pull request #1513: URL: https://github.com/apache/hadoop-ozone/pull/1513#issuecomment-715293590 @bshashikant Thanks for reviewing the PR! I have added a commit which addresses the comments. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] lokeshj1703 commented on a change in pull request #1513: HDDS-4366. SCM deletion service should delete configured number of blocks every interval.
lokeshj1703 commented on a change in pull request #1513:
URL: https://github.com/apache/hadoop-ozone/pull/1513#discussion_r510827330
##
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java
##
@@ -323,31 +322,54 @@ public void addTransactions(Map>
containerBlocksMap)
public void close() throws IOException {
}
+ private void getTransaction(DeletedBlocksTransaction tx,
Review comment:
The function is used to fetch a single transaction so avoided s at end
here.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] timmylicheng commented on a change in pull request #1514: HDDS-4191 Add failover proxy for SCM container location.
timmylicheng commented on a change in pull request #1514:
URL: https://github.com/apache/hadoop-ozone/pull/1514#discussion_r510791922
##
File path:
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMContainerLocationFailoverProxyProvider.java
##
@@ -0,0 +1,284 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.proxy;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import org.apache.hadoop.hdds.scm.ScmConfigKeys;
+import
org.apache.hadoop.hdds.scm.protocolPB.StorageContainerLocationProtocolPB;
+import org.apache.hadoop.hdds.utils.LegacyHadoopConfigurationSource;
+import org.apache.hadoop.io.retry.FailoverProxyProvider;
+import org.apache.hadoop.io.retry.RetryPolicy;
+import org.apache.hadoop.ipc.ProtobufRpcEngine;
+import org.apache.hadoop.ipc.RPC;
+import org.apache.hadoop.net.NetUtils;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+
+import static org.apache.hadoop.hdds.HddsUtils.getHostName;
+import static org.apache.hadoop.hdds.HddsUtils.getPortNumberFromConfigKeys;
+import static org.apache.hadoop.hdds.HddsUtils.getScmAddressForClients;
+import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_NAMES;
+import static
org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SERVICE_IDS_KEY;
+
+/**
+ * Failover proxy provider for SCM container location.
+ */
+public class SCMContainerLocationFailoverProxyProvider implements
+FailoverProxyProvider, Closeable {
+ public static final Logger LOG =
+ LoggerFactory.getLogger(StorageContainerLocationProtocolPB.class);
+
+ private Map>
scmProxies;
+ private Map scmProxyInfoMap;
+ private List scmNodeIDList;
+
+ private String currentProxySCMNodeId;
+ private int currentProxyIndex;
+
+ private final ConfigurationSource conf;
+ private final SCMClientConfig scmClientConfig;
+ private final long scmVersion;
+
+ private final String scmServiceId;
+
+ private final int maxRetryCount;
+ private final long retryInterval;
+
+ public static final String SCM_DUMMY_NODEID_PREFIX = "scm";
+
+ public SCMContainerLocationFailoverProxyProvider(ConfigurationSource conf) {
+this.conf = conf;
+this.scmVersion = RPC.getProtocolVersion(
+StorageContainerLocationProtocolPB.class);
+this.scmServiceId = conf.getTrimmed(OZONE_SCM_SERVICE_IDS_KEY);
Review comment:
Wanted to keep it as final variable so that value is only set in
constructor. What do you think?
##
File path:
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMContainerLocationFailoverProxyProvider.java
##
@@ -0,0 +1,284 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.proxy;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import org.apache.hadoop.hdds.scm.ScmConfigKeys;
+import
[GitHub] [hadoop-ozone] bshashikant opened a new pull request #1519: HDDS-4388. Make writeStateMachineTimeout retry count proportional to node failure timeout
bshashikant opened a new pull request #1519: URL: https://github.com/apache/hadoop-ozone/pull/1519 ## What changes were proposed in this pull request? Currently, in ratis "writeStateMachinecall" gets retried indefinitely in event of a timeout. In case, where disks are slow/overloaded or number of chunk writer threads are not available for a period of 10s, writeStateMachine call times out in 10s. In cases like these, the same write chunk keeps on getting retried causing the same chink of data to be overwritten. The idea here is to abort the request once the node failure timeout reaches. ## What is the link to the Apache JIRA https://issues.apache.org/jira/browse/HDDS-4388 ## How was this patch tested? Verified by checking the config value in tests. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] bshashikant commented on a change in pull request #1513: HDDS-4366. SCM deletion service should delete configured number of blocks every interval.
bshashikant commented on a change in pull request #1513:
URL: https://github.com/apache/hadoop-ozone/pull/1513#discussion_r510761184
##
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java
##
@@ -127,62 +112,65 @@ public int getPriority() {
@Override
public EmptyTaskResult call() throws Exception {
- int dnTxCount = 0;
long startTime = Time.monotonicNow();
// Scan SCM DB in HB interval and collect a throttled list of
// to delete blocks.
- LOG.debug("Running DeletedBlockTransactionScanner");
- DatanodeDeletedBlockTransactions transactions = null;
+ if (LOG.isDebugEnabled()) {
+LOG.debug("Running DeletedBlockTransactionScanner");
+ }
+
List datanodes =
nodeManager.getNodes(NodeState.HEALTHY);
- Map transactionMap = null;
if (datanodes != null) {
-transactions = new DatanodeDeletedBlockTransactions(containerManager,
-blockDeleteLimitSize, datanodes.size());
try {
- transactionMap = deletedBlockLog.getTransactions(transactions);
+ DatanodeDeletedBlockTransactions transactions =
+ deletedBlockLog.getTransactions(blockDeleteLimitSize);
+ Map containerIdToMaxTxnId =
+ transactions.getContainerIdToTxnIdMap();
+
+ if (transactions.isEmpty()) {
+return EmptyTaskResult.newResult();
+ }
+
+ for (Map.Entry> entry :
+ transactions.getDatanodeTransactionMap().entrySet()) {
+UUID dnId = entry.getKey();
+List dnTXs = entry.getValue();
+if (!dnTXs.isEmpty()) {
+ // TODO commandQueue needs a cap.
+ // We should stop caching new commands if num of un-processed
+ // command is bigger than a limit, e.g 50. In case datanode goes
+ // offline for sometime, the cached commands be flooded.
+ eventPublisher.fireEvent(SCMEvents.RETRIABLE_DATANODE_COMMAND,
+ new CommandForDatanode<>(dnId,
+ new DeleteBlocksCommand(dnTXs)));
+ if (LOG.isDebugEnabled()) {
+LOG.debug(
+"Added delete block command for datanode {} in the queue,"
++ " number of delete block transactions: {}{}", dnId,
+dnTXs.size(), LOG.isTraceEnabled() ?
+", TxID list: " + String.join(",",
+transactions.getTransactionIDList(dnId)) : "");
+ }
+}
+ }
+
+ containerManager.updateDeleteTransactionId(containerIdToMaxTxnId);
+ LOG.info("Totally added {} blocks to be deleted for"
+ + " {} datanodes, task elapsed time: {}ms",
+ transactions.getBlocksDeleted(),
+ transactions.getDatanodeTransactionMap().size(),
+ Time.monotonicNow() - startTime);
} catch (IOException e) {
// We may tolerant a number of failures for sometime
Review comment:
tolerant ---> tolerate
##
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java
##
@@ -323,31 +322,54 @@ public void addTransactions(Map>
containerBlocksMap)
public void close() throws IOException {
}
+ private void getTransaction(DeletedBlocksTransaction tx,
Review comment:
getTransaction -> getTransactions
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] GlenGeng opened a new pull request #1518: HDDS-4386: Each EndpointStateMachine uses its own thread pool to talk with SCM/Recon
GlenGeng opened a new pull request #1518:
URL: https://github.com/apache/hadoop-ozone/pull/1518
## What changes were proposed in this pull request?
In Tencent production environment, after start Recon for a while, we got
warnings that all DNs become stale/dead at SCM side. After kill recon, all DNs
become healthy in a very short time.
**The root cause is:**
1. EndpointStateMachine for SCM and that for Recon share the thread pool
created by DatanodeStateMachine, which is a fixed size thread pool:
```
executorService = Executors.newFixedThreadPool(
getEndPointTaskThreadPoolSize(),
new ThreadFactoryBuilder()
.setNameFormat("Datanode State Machine Task Thread - %d").build());
```
```
private int getEndPointTaskThreadPoolSize() {
// TODO(runzhiwang): current only support one recon, if support multiple
// recon in future reconServerCount should be the real number of recon
int reconServerCount = 1;
int totalServerCount = reconServerCount;
try {
totalServerCount += HddsUtils.getSCMAddresses(conf).size();
} catch (Exception e) {
LOG.error("Fail to get scm addresses", e);
}
return totalServerCount;
}
```
meanwhile, current Recon has some performance issue, after running for
hours, it became slower and slower, and crashed due to OOM.
2. The communication between DN and Recon will soon exhaust all the threads
in DatanodeStateMachine.executorService, there will be no available threads for
DN to talk SCM.
3. all DNs become stale/dead at SCM side.
**The fix is quite straightforward:**
Each EndpointStateMachine uses its own thread pool to talk with SCM/Recon, a
slow Recon won't interfere the communication between DN and SCM, or vice versa.
**P.S.**
The first edition for` DatanodeStateMachine.executorService` is a cached
thread pool, if there exists a slow SCM/Recon, more and more threads will be
created, and DN will OOM eventually, due to tens of thousands of threads are
created.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4386
Please replace this section with the link to the Apache JIRA)
CI
(Please explain how this patch was tested. Ex: unit tests, manual tests)
(If this patch involves UI changes, please attach a screen-shot; otherwise,
remove this)
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] GlenGeng closed pull request #1510: [Draft]HDDS-4191: failover proxy for container location protocol
GlenGeng closed pull request #1510: URL: https://github.com/apache/hadoop-ozone/pull/1510 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1514: HDDS-4191 Add failover proxy for SCM container location.
xiaoyuyao commented on a change in pull request #1514:
URL: https://github.com/apache/hadoop-ozone/pull/1514#discussion_r510581749
##
File path:
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMContainerLocationFailoverProxyProvider.java
##
@@ -0,0 +1,284 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.proxy;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import org.apache.hadoop.hdds.scm.ScmConfigKeys;
+import
org.apache.hadoop.hdds.scm.protocolPB.StorageContainerLocationProtocolPB;
+import org.apache.hadoop.hdds.utils.LegacyHadoopConfigurationSource;
+import org.apache.hadoop.io.retry.FailoverProxyProvider;
+import org.apache.hadoop.io.retry.RetryPolicy;
+import org.apache.hadoop.ipc.ProtobufRpcEngine;
+import org.apache.hadoop.ipc.RPC;
+import org.apache.hadoop.net.NetUtils;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+
+import static org.apache.hadoop.hdds.HddsUtils.getHostName;
+import static org.apache.hadoop.hdds.HddsUtils.getPortNumberFromConfigKeys;
+import static org.apache.hadoop.hdds.HddsUtils.getScmAddressForClients;
+import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_NAMES;
+import static
org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SERVICE_IDS_KEY;
+
+/**
+ * Failover proxy provider for SCM container location.
+ */
+public class SCMContainerLocationFailoverProxyProvider implements
+FailoverProxyProvider, Closeable {
+ public static final Logger LOG =
+ LoggerFactory.getLogger(StorageContainerLocationProtocolPB.class);
+
+ private Map>
scmProxies;
+ private Map scmProxyInfoMap;
+ private List scmNodeIDList;
+
+ private String currentProxySCMNodeId;
+ private int currentProxyIndex;
+
+ private final ConfigurationSource conf;
+ private final SCMClientConfig scmClientConfig;
+ private final long scmVersion;
+
+ private final String scmServiceId;
+
+ private final int maxRetryCount;
+ private final long retryInterval;
+
+ public static final String SCM_DUMMY_NODEID_PREFIX = "scm";
+
+ public SCMContainerLocationFailoverProxyProvider(ConfigurationSource conf) {
+this.conf = conf;
+this.scmVersion = RPC.getProtocolVersion(
+StorageContainerLocationProtocolPB.class);
+this.scmServiceId = conf.getTrimmed(OZONE_SCM_SERVICE_IDS_KEY);
Review comment:
Can we move line 82 into loadConfigs()?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1516: HDDS-3731. [doc]add storage space quota document.
captainzmc commented on pull request #1516: URL: https://github.com/apache/hadoop-ozone/pull/1516#issuecomment-714884328 Thanks @xiaoyuyao 's review. I have submitted a new commit fix the above issues. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1516: HDDS-3731. [doc]add storage space quota document.
captainzmc commented on a change in pull request #1516: URL: https://github.com/apache/hadoop-ozone/pull/1516#discussion_r510568534 ## File path: hadoop-hdds/docs/content/feature/Quota.md ## @@ -0,0 +1,67 @@ +--- +title: "Quota in Ozone" +date: "2020-October-22" +weight: 4 +summary: Quota in Ozone +icon: user +menu: + main: + parent: Features +summary: Introduction to Ozone Quota +--- + + +So far, we know that Ozone allows users to create volumes, buckets, and keys. A Volume usually contains several buckets, and each Bucket also contains a certain number of keys. Obviously, it should allow the user to define quotas (for example, how many buckets can be created under a Volume or how much space can be used by a Bucket), which is a common requirement for storage systems. + +## Currently supported +1. Storage Space level quota Review comment: The name space Quota feature is currently not available and is under development. We can add this after it is complete. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1516: HDDS-3731. [doc]add storage space quota document.
captainzmc commented on a change in pull request #1516: URL: https://github.com/apache/hadoop-ozone/pull/1516#discussion_r510568737 ## File path: hadoop-hdds/docs/content/feature/Quota.md ## @@ -0,0 +1,67 @@ +--- +title: "Quota in Ozone" +date: "2020-October-22" +weight: 4 +summary: Quota in Ozone +icon: user +menu: + main: + parent: Features +summary: Introduction to Ozone Quota +--- + + +So far, we know that Ozone allows users to create volumes, buckets, and keys. A Volume usually contains several buckets, and each Bucket also contains a certain number of keys. Obviously, it should allow the user to define quotas (for example, how many buckets can be created under a Volume or how much space can be used by a Bucket), which is a common requirement for storage systems. + +## Currently supported +1. Storage Space level quota + +Administrators should be able to define how much storage space a Volume or Bucket can use. + +## Client usage +### Storage Space level quota +Storage space level quotas allow the use of units such as KB (k), MB (m), GB (g), TB (t), PB (p), etc. Represents how many storage Spaces will be used. + + Volume Storage Space level quota +```shell +bin/ozone sh volume create --space-quota 5m /volume1 +``` +This means setting the storage space of Volume1 to 5MB + +```shell +bin/ozone sh volume setquota --space-quota 10g /volume1 +``` +This behavior changes the quota of Volume1 to 10GB. + + Bucket Storage Space level quota +```shell +bin/ozone sh bucket create --space-quota 5m /volume1/bucket1 +``` +That means bucket1 allows us to use 5MB of storage. + +```shell +bin/ozone sh bucket setquota --space-quota 10g /volume1/bucket1 +``` +This behavior changes the quota for Bucket1 to 10GB + +A bucket quota should not be greater than its Volume quota. Let's look at an example. If we have a 10MB Volume and create five buckets under that Volume with a quota of 5MB, the total quota is 25MB. In this case, the bucket creation will always succeed, and we check the quota for bucket and volume when the data is actually written. Each write needs to check whether the current bucket is exceeding the limit and the current total volume usage is exceeding the limit. Review comment: Yes, the quota value of bucket cannot be greater than the quota value of volume This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
