You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by "stefan-egli (via GitHub)" <gi...@apache.org> on 2024/07/03 16:16:25 UTC
[PR] OAK-10933 : skip timestamp calculation if fullgc is not enabled [jackrabbit-oak]
stefan-egli opened a new pull request, #1570:
URL: https://github.com/apache/jackrabbit-oak/pull/1570
(no comment)
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10933 : skip timestamp calculation if fullgc is not enabled [jackrabbit-oak]
Posted by "stefan-egli (via GitHub)" <gi...@apache.org>.
stefan-egli commented on PR #1570:
URL: https://github.com/apache/jackrabbit-oak/pull/1570#issuecomment-2208485300
tests look "good"
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10933 : skip timestamp calculation if fullgc is not enabled [jackrabbit-oak]
Posted by "rishabhdaim (via GitHub)" <gi...@apache.org>.
rishabhdaim commented on PR #1570:
URL: https://github.com/apache/jackrabbit-oak/pull/1570#issuecomment-2208614324
IMO, this is an incomplete fix. We need to add `fullGCEnabled` check before `isFullGCDryRun` at https://github.com/apache/jackrabbit-oak/blob/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java#L152 & https://github.com/apache/jackrabbit-oak/blob/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java#L152, just like we did here: https://github.com/apache/jackrabbit-oak/blob/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java#L304
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10933 : skip timestamp calculation if fullgc is not enabled [jackrabbit-oak]
Posted by "stefan-egli (via GitHub)" <gi...@apache.org>.
stefan-egli commented on PR #1570:
URL: https://github.com/apache/jackrabbit-oak/pull/1570#issuecomment-2208700186
agree this is incomplete - it is merely a first fix for the most noisy logger. we likely will have to exclude more - but that shouldn't prevent this one from going through.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10933 : skip timestamp calculation if fullgc is not enabled [jackrabbit-oak]
Posted by "stefan-egli (via GitHub)" <gi...@apache.org>.
stefan-egli merged PR #1570:
URL: https://github.com/apache/jackrabbit-oak/pull/1570
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10933 : skip timestamp calculation if fullgc is not enabled [jackrabbit-oak]
Posted by "rishabhdaim (via GitHub)" <gi...@apache.org>.
rishabhdaim commented on code in PR #1570:
URL: https://github.com/apache/jackrabbit-oak/pull/1570#discussion_r1665485930
##########
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java:
##########
@@ -170,7 +170,7 @@ public class VersionGCRecommendations {
} else {
oldestModifiedDryRunDocTimeStamp.set(fullGCDryRunTimestamp);
}
- } else {
+ } else if (fullGCEnabled) {
Review Comment:
We should move this check before the `isFullGCDryRun` check.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10933 : skip timestamp calculation if fullgc is not enabled [jackrabbit-oak]
Posted by "stefan-egli (via GitHub)" <gi...@apache.org>.
stefan-egli commented on code in PR #1570:
URL: https://github.com/apache/jackrabbit-oak/pull/1570#discussion_r1665537149
##########
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGCRecommendations.java:
##########
@@ -170,7 +170,7 @@ public class VersionGCRecommendations {
} else {
oldestModifiedDryRunDocTimeStamp.set(fullGCDryRunTimestamp);
}
- } else {
+ } else if (fullGCEnabled) {
Review Comment:
I was assuming dry run mode wouldn't be enabled if fullgc is disabled - but probably better to protect from that indeed. now done in https://github.com/apache/jackrabbit-oak/pull/1570/commits/0777573384e8d6504ab06eda073d0805a4e8a53a
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-10933 : skip timestamp calculation if fullgc is not enabled [jackrabbit-oak]
Posted by "stefan-egli (via GitHub)" <gi...@apache.org>.
stefan-egli commented on PR #1570:
URL: https://github.com/apache/jackrabbit-oak/pull/1570#issuecomment-2206737281
PS: waiting for test to confirm no regression introduced by this - but ideally this would be included asap
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org