You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "szetszwo (via GitHub)" <gi...@apache.org> on 2023/02/08 01:12:54 UTC
[GitHub] [ozone] szetszwo opened a new pull request, #4255: HDDS-7782. OM lease recovery for hsync'ed files.
szetszwo opened a new pull request, #4255:
URL: https://github.com/apache/ozone/pull/4255
## What changes were proposed in this pull request?
When an OPEN file is abandoned by the original client up to the lease hard limit (7 days by default):
- If the file is not hsync’ed, delete it. (same as today)
- If the file is hsync’ed, close it at the last hsync’ed length.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7782
## How was this patch tested?
Modified existing tests and new 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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] szetszwo commented on pull request #4255: HDDS-7782. OM lease recovery for hsync'ed files.
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #4255:
URL: https://github.com/apache/ozone/pull/4255#issuecomment-1424714959
@Galsza , thanks for reviewing this. I am adding some new tests for the hsync'ed files.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] szetszwo commented on pull request #4255: HDDS-7782. OM lease recovery for hsync'ed files.
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #4255:
URL: https://github.com/apache/ozone/pull/4255#issuecomment-1449029714
@Galsza , @sumitagrawl , @jojochuang , thanks a lot for reviewing 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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] Galsza commented on pull request #4255: HDDS-7782. OM lease recovery for hsync'ed files.
Posted by "Galsza (via GitHub)" <gi...@apache.org>.
Galsza commented on PR #4255:
URL: https://github.com/apache/ozone/pull/4255#issuecomment-1424424054
Thanks for the patch, overall looks good to me. One minor thing: could you add a test case for the closing of the keys?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] szetszwo commented on a diff in pull request #4255: HDDS-7782. OM lease recovery for hsync'ed files.
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #4255:
URL: https://github.com/apache/ozone/pull/4255#discussion_r1106276701
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -1411,33 +1415,49 @@ public List<OpenKeyBucket> getExpiredOpenKeys(Duration expireThreshold,
final long expiredCreationTimestamp =
expireThreshold.negated().plusMillis(Time.now()).toMillis();
- OpenKey.Builder builder = OpenKey.newBuilder();
int num = 0;
while (num < count && keyValueTableIterator.hasNext()) {
KeyValue<String, OmKeyInfo> openKeyValue = keyValueTableIterator.next();
String dbOpenKeyName = openKeyValue.getKey();
+
+ final int lastPrefix = dbOpenKeyName.lastIndexOf(OM_KEY_PREFIX);
+ final String dbKeyName = dbOpenKeyName.substring(0, lastPrefix);
OmKeyInfo openKeyInfo = openKeyValue.getValue();
if (openKeyInfo.getCreationTime() <= expiredCreationTimestamp) {
- final String volume = openKeyInfo.getVolumeName();
- final String bucket = openKeyInfo.getBucketName();
- final String mapKey = volume + OM_KEY_PREFIX + bucket;
- if (!expiredKeys.containsKey(mapKey)) {
- expiredKeys.put(mapKey,
- OpenKeyBucket.newBuilder()
- .setVolumeName(volume)
- .setBucketName(bucket));
+ final OmKeyInfo info = kt.get(dbKeyName);
+
+ if (info == null) {
+ // add non-hsync'ed keys
+ expiredKeys.addOpenKey(openKeyInfo, dbOpenKeyName);
+ } else {
+ // add hsync'ed keys
+ final KeyArgs.Builder keyArgs = KeyArgs.newBuilder()
Review Comment:
@sumitagrawl , thanks for taking a look! You are right that the `info` obtained may be from a different key closed earlier.
We should put hsync client id in the hsync'ed key. Then, we can tell the difference.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] szetszwo commented on pull request #4255: HDDS-7782. OM lease recovery for hsync'ed files.
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #4255:
URL: https://github.com/apache/ozone/pull/4255#issuecomment-1431818996
Sync'ed with master after the TestHSync change by #4190 .
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] szetszwo commented on pull request #4255: HDDS-7782. OM lease recovery for hsync'ed files.
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #4255:
URL: https://github.com/apache/ozone/pull/4255#issuecomment-1433848930
@jojochuang , thanks for reviewing this!
> How about adding metrics to keep track of numOpenKeys and numHsyncKeys?
Sure. Let's add the metrics in https://issues.apache.org/jira/browse/HDDS-7855
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sumitagrawl commented on a diff in pull request #4255: HDDS-7782. OM lease recovery for hsync'ed files.
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4255:
URL: https://github.com/apache/ozone/pull/4255#discussion_r1105757787
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -1411,33 +1415,49 @@ public List<OpenKeyBucket> getExpiredOpenKeys(Duration expireThreshold,
final long expiredCreationTimestamp =
expireThreshold.negated().plusMillis(Time.now()).toMillis();
- OpenKey.Builder builder = OpenKey.newBuilder();
int num = 0;
while (num < count && keyValueTableIterator.hasNext()) {
KeyValue<String, OmKeyInfo> openKeyValue = keyValueTableIterator.next();
String dbOpenKeyName = openKeyValue.getKey();
+
+ final int lastPrefix = dbOpenKeyName.lastIndexOf(OM_KEY_PREFIX);
+ final String dbKeyName = dbOpenKeyName.substring(0, lastPrefix);
OmKeyInfo openKeyInfo = openKeyValue.getValue();
if (openKeyInfo.getCreationTime() <= expiredCreationTimestamp) {
- final String volume = openKeyInfo.getVolumeName();
- final String bucket = openKeyInfo.getBucketName();
- final String mapKey = volume + OM_KEY_PREFIX + bucket;
- if (!expiredKeys.containsKey(mapKey)) {
- expiredKeys.put(mapKey,
- OpenKeyBucket.newBuilder()
- .setVolumeName(volume)
- .setBucketName(bucket));
+ final OmKeyInfo info = kt.get(dbKeyName);
+
+ if (info == null) {
+ // add non-hsync'ed keys
+ expiredKeys.addOpenKey(openKeyInfo, dbOpenKeyName);
+ } else {
+ // add hsync'ed keys
+ final KeyArgs.Builder keyArgs = KeyArgs.newBuilder()
Review Comment:
There is no distinction if key needs to be applied for hsync OR its to be rejected as explicit commit is not performed.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] jojochuang merged pull request #4255: HDDS-7782. OM lease recovery for hsync'ed files.
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang merged PR #4255:
URL: https://github.com/apache/ozone/pull/4255
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org