[GitHub] athanatos opened a new pull request #275: BOOKKEEPER-1102: Fix BookKeeeperDiskSpaceWeightedLedgerPlacementTest

[gi...@git.apache.org]

athanatos opened a new pull request #275: BOOKKEEPER-1102: Fix BookKeeeperDiskSpaceWeightedLedgerPlacementTest
URL: https://github.com/apache/bookkeeper/pull/275
 
 
   BookieInfoReader:
   
   The previous syncronization logic wasn't really correct, and the logic
   at the top of the method was far more complicated than it needed to be.
   Restrict bookies to be non-null.  Restructure the code to simply use
   the BookieInfoReader instance as a single lock.
   
   One significant behavioral change is that we scan every bookie not in
   the map, and we clear from the map bookies which returned an error.
   
   Also, explicitely cache the most recent bookie set reported by the
   BookieWatcher.  This eliminates the need to call into BookieWatcher
   from getReadWriteBookieInfo and the corresponding error path.  The
   periodic scan continues to explicitely check.
   
   Another departure is the addition of an explicit retry-on-error param to
   trigger retry if any of the requests failed
   (getBookieInfoRetryIntervalSeconds).  We'll only retry the ones that
   actually failed (along with any new additions since the last run).  This
   is useful because bookie startup triggers the addition of the bookie
   node to zk before the bookie actually becomes available for the bookie
   info request, so there can be rare races in the unit tests between
   BookieInfoReader requesting the info and the bookie actually being up.
   
   Also, add a method to allow tests to wait for updates to be reflected.
   
   PerChannelBookieClient: fix error handling for BookieInfo keys
   
   Passing a key corresponding to a GET_BOOKIE_INFO operation to
   errorOutReadKey results in a casting exception, clean up the invalid
   calls.
   
   BookKeeperClusterTestCase: add killBookieAndWaitForZK
   
   Should reduce the need for tests to wait for an arbitrary period to let
   the cluster "settle".
   
   BookKeeperDiskSpaceWeightedLedgerPlacementTest:
   
   This test was heavily time dependent, and the Thread.sleep values did
   not work universally.  Instead, eliminate the arbitrary Thread.sleep
   values and instead verify the free space changes on each change.
   
   Also, switch the delay on
   testDiskSpaceWeightedBookieSelectionWithPeriodicBookieInfoUpdate
   to simply use an atomic boolean to signal the value switch.
   
   Signed-off-by: Samuel Just <sj...@salesforce.com>
   
   Descriptions of the changes in this PR:
   
   (PR description content here)...
   
   ---
   Be sure to do all of the following to help us incorporate your contribution
   quickly and easily:
   
   - [ ] Make sure the PR title is formatted like:
       `<Issue # or BOOKKEEPER-#>: Description of pull request`
       `e.g. Issue 123: Description ...`
       `e.g. BOOKKEEPER-1234: Description ...`
   - [ ] Make sure tests pass via `mvn clean apache-rat:check install findbugs:check`.
   - [ ] Replace `<Issue # or BOOKKEEPER-#>` in the title with the actual Issue/JIRA number.
   
   ---
   
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services