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/08/25 21:12:06 UTC
[GitHub] [hadoop-ozone] errose28 opened a new pull request #1351: HDDS-4121. Implement OmMetadataMangerImpl#getExpiredOpenKeys.
errose28 opened a new pull request #1351:
URL: https://github.com/apache/hadoop-ozone/pull/1351
## What changes were proposed in this pull request?
Create an implementation to the existing OmMetadataMangerImpl#getExpiredOpenKeys method stub as the first step towards completing parent Jira HDDS-4120. This method returns a list of key information for all keys in the OM open key table whose creation time is older than a time interval configurable with the existing ozone.open.key.expire.threshold setting. The existing default value for this setting (one day) was not changed.
## What is the link to the Apache JIRA
HDDS-4121
## How was this patch tested?
The unit test TestOmMetadataManager#testGetExpiredOpenKeys was added, which creates a mixture of expired and unexpired open keys, and tests whether the expired open keys are read correctly. Functionality was added to TestOMRequestUtils to allow setting customized creation times for keys to simulate expiration.
----------------------------------------------------------------
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] bharatviswa504 commented on a change in pull request #1351: HDDS-4121. Implement OmMetadataMangerImpl#getExpiredOpenKeys.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1351:
URL: https://github.com/apache/hadoop-ozone/pull/1351#discussion_r476742648
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManager.java
##########
@@ -186,10 +186,11 @@ OmKeyLocationInfo allocateBlock(OmKeyArgs args, long clientID,
* the key name and all its associated block IDs. A pending open key has
* prefix #open# in OM DB.
*
+ * @param count The maximum number of expired opne keys to return.
Review comment:
opne -> open
----------------------------------------------------------------
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] errose28 commented on a change in pull request #1351: HDDS-4121. Implement OmMetadataMangerImpl#getExpiredOpenKeys.
Posted by GitBox <gi...@apache.org>.
errose28 commented on a change in pull request #1351:
URL: https://github.com/apache/hadoop-ozone/pull/1351#discussion_r477334837
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManager.java
##########
@@ -186,10 +186,11 @@ OmKeyLocationInfo allocateBlock(OmKeyArgs args, long clientID,
* the key name and all its associated block IDs. A pending open key has
* prefix #open# in OM DB.
Review comment:
Is this line in the original documentation still relevant? It looks like open keys are placed in the open key table unprefixed.
----------------------------------------------------------------
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] bharatviswa504 commented on a change in pull request #1351: HDDS-4121. Implement OmMetadataMangerImpl#getExpiredOpenKeys.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1351:
URL: https://github.com/apache/hadoop-ozone/pull/1351#discussion_r479506351
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java
##########
@@ -521,6 +527,80 @@ public void testListKeysWithFewDeleteEntriesInCache() throws Exception {
}
+ @Test
+ public void testGetExpiredOpenKeys() throws Exception {
+ final String bucketName = "bucket";
+ final String volumeName = "volume";
+ final int numExpiredOpenKeys = 4;
+ final int numUnexpiredOpenKeys = 1;
+ final long clientID = 1000L;
+ // To create expired keys, they will be assigned a creation time twice as
+ // old as the minimum expiration time.
+ final long minExpiredTimeSeconds = ozoneConfiguration.getInt(
+ OZONE_OPEN_KEY_EXPIRE_THRESHOLD_SECONDS,
+ OZONE_OPEN_KEY_EXPIRE_THRESHOLD_SECONDS_DEFAULT);
+ final long expiredAgeMillis =
+ Instant.now().minus(minExpiredTimeSeconds * 2,
+ ChronoUnit.SECONDS).toEpochMilli();
+
+ // Add expired keys to open key table.
+ // The method under test does not check for expired open keys in the
+ // cache, since they will be picked up once the cache is flushed.
+ Set<String> expiredKeys = new HashSet<>();
+ for (int i = 0; i < numExpiredOpenKeys; i++) {
+ OmKeyInfo keyInfo = TestOMRequestUtils.createOmKeyInfo(volumeName,
+ bucketName, "expired" + i, HddsProtos.ReplicationType.RATIS,
+ HddsProtos.ReplicationFactor.ONE, 0L, expiredAgeMillis);
+
+ TestOMRequestUtils.addKeyToTable(true, false,
+ keyInfo, clientID, 0L, omMetadataManager);
+
+ String groupID = omMetadataManager.getOpenKey(volumeName, bucketName,
+ keyInfo.getKeyName(), clientID);
+ expiredKeys.add(groupID);
+ }
+
+ // Add unexpired keys to open key table.
+ for (int i = 0; i < numUnexpiredOpenKeys; i++) {
+ OmKeyInfo keyInfo = TestOMRequestUtils.createOmKeyInfo(volumeName,
+ bucketName, "unexpired" + i, HddsProtos.ReplicationType.RATIS,
+ HddsProtos.ReplicationFactor.ONE);
+
+ TestOMRequestUtils.addKeyToTable(true, false,
+ keyInfo, clientID, 0L, omMetadataManager);
+
+ String groupID = omMetadataManager.getOpenKey(volumeName, bucketName,
Review comment:
Unused code 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:
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] bharatviswa504 commented on pull request #1351: HDDS-4121. Implement OmMetadataMangerImpl#getExpiredOpenKeys.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1351:
URL: https://github.com/apache/hadoop-ozone/pull/1351#issuecomment-684063895
Thank You @errose28 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:
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] bharatviswa504 commented on a change in pull request #1351: HDDS-4121. Implement OmMetadataMangerImpl#getExpiredOpenKeys.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1351:
URL: https://github.com/apache/hadoop-ozone/pull/1351#discussion_r476754181
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManager.java
##########
@@ -186,10 +186,11 @@ OmKeyLocationInfo allocateBlock(OmKeyArgs args, long clientID,
* the key name and all its associated block IDs. A pending open key has
* prefix #open# in OM DB.
*
+ * @param count The maximum number of expired opne keys to return.
* @return a list of {@link BlockGroup} representing keys and blocks.
* @throws IOException
*/
- List<BlockGroup> getExpiredOpenKeys() throws IOException;
+ List<BlockGroup> getExpiredOpenKeys(int count) throws IOException;
Review comment:
You can ignore my 2nd comment, as delete expiredKeys will be done in a single iteration, those expired keys will not be there in the openKeyTable.
----------------------------------------------------------------
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] bharatviswa504 commented on a change in pull request #1351: HDDS-4121. Implement OmMetadataMangerImpl#getExpiredOpenKeys.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1351:
URL: https://github.com/apache/hadoop-ozone/pull/1351#discussion_r476741653
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManager.java
##########
@@ -186,10 +186,11 @@ OmKeyLocationInfo allocateBlock(OmKeyArgs args, long clientID,
* the key name and all its associated block IDs. A pending open key has
* prefix #open# in OM DB.
*
+ * @param count The maximum number of expired opne keys to return.
* @return a list of {@link BlockGroup} representing keys and blocks.
* @throws IOException
*/
- List<BlockGroup> getExpiredOpenKeys() throws IOException;
+ List<BlockGroup> getExpiredOpenKeys(int count) throws IOException;
Review comment:
Here, we can collect just expired KeyNames.
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManager.java
##########
@@ -186,10 +186,11 @@ OmKeyLocationInfo allocateBlock(OmKeyArgs args, long clientID,
* the key name and all its associated block IDs. A pending open key has
* prefix #open# in OM DB.
*
+ * @param count The maximum number of expired opne keys to return.
* @return a list of {@link BlockGroup} representing keys and blocks.
* @throws IOException
*/
- List<BlockGroup> getExpiredOpenKeys() throws IOException;
+ List<BlockGroup> getExpiredOpenKeys(int count) throws IOException;
Review comment:
And also I think we might need last expired key also, as in next iteration we need to start from that one.
----------------------------------------------------------------
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] bharatviswa504 merged pull request #1351: HDDS-4121. Implement OmMetadataMangerImpl#getExpiredOpenKeys.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 merged pull request #1351:
URL: https://github.com/apache/hadoop-ozone/pull/1351
----------------------------------------------------------------
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] errose28 commented on a change in pull request #1351: HDDS-4121. Implement OmMetadataMangerImpl#getExpiredOpenKeys.
Posted by GitBox <gi...@apache.org>.
errose28 commented on a change in pull request #1351:
URL: https://github.com/apache/hadoop-ozone/pull/1351#discussion_r479513426
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java
##########
@@ -521,6 +527,80 @@ public void testListKeysWithFewDeleteEntriesInCache() throws Exception {
}
+ @Test
+ public void testGetExpiredOpenKeys() throws Exception {
+ final String bucketName = "bucket";
+ final String volumeName = "volume";
+ final int numExpiredOpenKeys = 4;
+ final int numUnexpiredOpenKeys = 1;
+ final long clientID = 1000L;
+ // To create expired keys, they will be assigned a creation time twice as
+ // old as the minimum expiration time.
+ final long minExpiredTimeSeconds = ozoneConfiguration.getInt(
+ OZONE_OPEN_KEY_EXPIRE_THRESHOLD_SECONDS,
+ OZONE_OPEN_KEY_EXPIRE_THRESHOLD_SECONDS_DEFAULT);
+ final long expiredAgeMillis =
+ Instant.now().minus(minExpiredTimeSeconds * 2,
+ ChronoUnit.SECONDS).toEpochMilli();
+
+ // Add expired keys to open key table.
+ // The method under test does not check for expired open keys in the
+ // cache, since they will be picked up once the cache is flushed.
+ Set<String> expiredKeys = new HashSet<>();
+ for (int i = 0; i < numExpiredOpenKeys; i++) {
+ OmKeyInfo keyInfo = TestOMRequestUtils.createOmKeyInfo(volumeName,
+ bucketName, "expired" + i, HddsProtos.ReplicationType.RATIS,
+ HddsProtos.ReplicationFactor.ONE, 0L, expiredAgeMillis);
+
+ TestOMRequestUtils.addKeyToTable(true, false,
+ keyInfo, clientID, 0L, omMetadataManager);
+
+ String groupID = omMetadataManager.getOpenKey(volumeName, bucketName,
+ keyInfo.getKeyName(), clientID);
+ expiredKeys.add(groupID);
+ }
+
+ // Add unexpired keys to open key table.
+ for (int i = 0; i < numUnexpiredOpenKeys; i++) {
+ OmKeyInfo keyInfo = TestOMRequestUtils.createOmKeyInfo(volumeName,
+ bucketName, "unexpired" + i, HddsProtos.ReplicationType.RATIS,
+ HddsProtos.ReplicationFactor.ONE);
+
+ TestOMRequestUtils.addKeyToTable(true, false,
+ keyInfo, clientID, 0L, omMetadataManager);
+
+ String groupID = omMetadataManager.getOpenKey(volumeName, bucketName,
Review comment:
Yeah. Removed it and pushing again.
----------------------------------------------------------------
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