You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/10/13 07:46:01 UTC
[GitHub] [hadoop-ozone] captainzmc opened a new pull request #1489: HDDS-4308. Fix issue with quota update
captainzmc opened a new pull request #1489:
URL: https://github.com/apache/hadoop-ozone/pull/1489
## What changes were proposed in this pull request?
Currently volumeArgs using getCacheValue and put the same object in doubleBuffer, this might cause issue.
Let's take the below scenario:
InitialVolumeArgs quotaBytes -> 10000
1. T1 -> Update VolumeArgs, and subtracting 1000 and put this updated volumeArgs to DoubleBuffer.
2. T2-> Update VolumeArgs, and subtracting 2000 and has not still updated to double buffer.
Now at the end of flushing these transactions, our DB should have 7000 as bytes used.
Now T1 is picked by double Buffer and when it commits, and as it uses cached Object put into doubleBuffer, it flushes to DB with the updated value from T2(As it is a cache object) and update DB with bytesUsed as 7000.
And now OM has restarted, and only DB has transactions till T1. (We get this info from TransactionInfo Table(https://issues.apache.org/jira/browse/HDDS-3685)
Now T2 is again replayed, as it is not committed to DB, now DB will be again subtracted with 2000, and now DB will have 5000.
But after T2, the value should be 7000, so we have DB in an incorrect state.
Issue here:
1. As we use a cached object and put the same cached object into double buffer this can cause this kind of issue.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4308
## How was this patch tested?
Use the current UT
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1489: HDDS-4308. Fix issue with quota update
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc commented on pull request #1489: HDDS-4308. Fix issue with quota update
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1489:
URL: https://github.com/apache/hadoop-ozone/pull/1489#issuecomment-708182129
Hi @bharatviswa504, Please 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] linyiqun commented on a change in pull request #1489: HDDS-4308. Fix issue with quota update
Posted by GitBox <gi...@apache.org>.
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<OmVolumeArgs> 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] captainzmc edited a comment on pull request #1489: HDDS-4308. Fix issue with quota update
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] linyiqun commented on a change in pull request #1489: HDDS-4308. Fix issue with quota update
Posted by GitBox <gi...@apache.org>.
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<OmVolumeArgs> 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org