You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2021/03/30 03:25:42 UTC

[GitHub] [bookkeeper] klwilson227 opened a new pull request #2668: Fix logic in Bookkeeper forceAllowCompaction to run only when force i…

klwilson227 opened a new pull request #2668:
URL: https://github.com/apache/bookkeeper/pull/2668


   Without the attached fix, when forceAllowCompaction and isForceGCAllowWhenNoSpace are set. The majorCompaction runs with every garbage collection. This results due to the fact that enableMajorCompaction which takes a negative interval into account to disable compaction always returns true since the difference in time will always be larger than -1. The time check must be "and" with the enableMajorCompaciton for the time check to be valid. And the force flag should also only be checked when the isForceGCAllowWhenNoSpace is activated. The attached PR makes the necessary adjustments. 


-- 
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



[GitHub] [bookkeeper] klwilson227 edited a comment on pull request #2668: Fix logic in Bookkeeper forceAllowCompaction to run only when force i…

Posted by GitBox <gi...@apache.org>.
klwilson227 edited a comment on pull request #2668:
URL: https://github.com/apache/bookkeeper/pull/2668#issuecomment-810682495


   Test Cases fixed up. 


-- 
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



[GitHub] [bookkeeper] klwilson227 commented on pull request #2668: Fix logic in Bookkeeper forceAllowCompaction to run only when force i…

Posted by GitBox <gi...@apache.org>.
klwilson227 commented on pull request #2668:
URL: https://github.com/apache/bookkeeper/pull/2668#issuecomment-810682495


   I’ll take a look.


-- 
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



[GitHub] [bookkeeper] klwilson227 closed pull request #2668: Fix logic in Bookkeeper forceAllowCompaction to run only when force i…

Posted by GitBox <gi...@apache.org>.
klwilson227 closed pull request #2668:
URL: https://github.com/apache/bookkeeper/pull/2668


   


-- 
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



[GitHub] [bookkeeper] klwilson227 commented on pull request #2668: Fix logic in Bookkeeper forceAllowCompaction to run only when force i…

Posted by GitBox <gi...@apache.org>.
klwilson227 commented on pull request #2668:
URL: https://github.com/apache/bookkeeper/pull/2668#issuecomment-811504349


   Re-opening new PR, as this does not have the intended update after rebase to squash extra commits.


-- 
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



[GitHub] [bookkeeper] eolivelli commented on pull request #2668: Fix logic in Bookkeeper forceAllowCompaction to run only when force i…

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #2668:
URL: https://github.com/apache/bookkeeper/pull/2668#issuecomment-810187311


   thank you very much for this patch
   
   There are test failures, can you please check?
   
   for instance:
   ```
   Error:  org.apache.bookkeeper.bookie.CompactionByEntriesTest.testForceGarbageCollection  Time elapsed: 0.941 s  <<< FAILURE!
   java.lang.AssertionError: Minor or major compaction did not trigger even on forcing.
   	at org.junit.Assert.fail(Assert.java:88)
   	at org.junit.Assert.assertTrue(Assert.java:41)
   	at org.apache.bookkeeper.bookie.CompactionTest.lambda$testForceGarbageCollection$0(CompactionTest.java:312)
   	at org.apache.bookkeeper.meta.MetadataDrivers.lambda$runFunctionWithLedgerManagerFactory$2(MetadataDrivers.java:389)
   	at org.apache.bookkeeper.meta.MetadataDrivers.runFunctionWithMetadataBookieDriver(MetadataDrivers.java:347)
   	at org.apache.bookkeeper.meta.MetadataDrivers.runFunctionWithLedgerManagerFactory(MetadataDrivers.java:387)
   	at org.apache.bookkeeper.bookie.CompactionTest.testForceGarbageCollection(CompactionTest.java:292)
   	at org.apache.bookkeeper.bookie.CompactionTest.testForceGarbageCollection(CompactionTest.java:255)
   	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
   ```
   
   also, can you please verify if there is a test that covers your change, or add a new test case ?


-- 
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