[jira] [Commented] (BOOKKEEPER-661) Turn readonly back to writable if spaces are reclaimed.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-661?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13877609#comment-13877609 ] Hudson commented on BOOKKEEPER-661: --- SUCCESS: Integrated in bookkeeper-trunk #520 (See [https://builds.apache.org/job/bookkeeper-trunk/520/]) BOOKKEEPER-661: Turn readonly back to writable if spaces are reclaimed. (sijie via ivank) (ivank: rev 1560066) * /zookeeper/bookkeeper/trunk/CHANGES.txt * /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java * /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java * /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java * /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexPersistenceMgr.java * /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java * /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsManager.java * /zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieInitializationTest.java * /zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java * /zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/TestSyncThread.java * /zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/ReadOnlyBookieTest.java Turn readonly back to writable if spaces are reclaimed. --- Key: BOOKKEEPER-661 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-661 Project: Bookkeeper Issue Type: Improvement Components: bookkeeper-server Reporter: Sijie Guo Assignee: Sijie Guo Fix For: 4.3.0 Attachments: BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661_662.diff should be able to turn a bookie from readonly back to writable if the spaces are reclaimed. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (BOOKKEEPER-661) Turn readonly back to writable if spaces are reclaimed.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-661?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13877167#comment-13877167 ] Hadoop QA commented on BOOKKEEPER-661: -- Testing JIRA BOOKKEEPER-661 Patch [BOOKKEEPER-661.diff|https://issues.apache.org/jira/secure/attachment/12624063/BOOKKEEPER-661.diff] downloaded at Tue Jan 21 04:10:58 UTC 2014 {color:red}-1{color} Patch failed to apply to head of branch Turn readonly back to writable if spaces are reclaimed. --- Key: BOOKKEEPER-661 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-661 Project: Bookkeeper Issue Type: Improvement Components: bookkeeper-server Reporter: Sijie Guo Assignee: Sijie Guo Fix For: 4.3.0 Attachments: BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661_662.diff should be able to turn a bookie from readonly back to writable if the spaces are reclaimed. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (BOOKKEEPER-661) Turn readonly back to writable if spaces are reclaimed.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-661?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13873180#comment-13873180 ] Hadoop QA commented on BOOKKEEPER-661: -- Testing JIRA BOOKKEEPER-661 Patch [BOOKKEEPER-661.diff|https://issues.apache.org/jira/secure/attachment/12623334/BOOKKEEPER-661.diff] downloaded at Thu Jan 16 08:40:47 UTC 2014 {color:green}+1 PATCH_APPLIES{color} {color:green}+1 CLEAN{color} {color:green}+1 RAW_PATCH_ANALYSIS{color} .{color:green}+1{color} the patch does not introduce any @author tags .{color:green}+1{color} the patch does not introduce any tabs .{color:green}+1{color} the patch does not introduce any trailing spaces .{color:green}+1{color} the patch does not introduce any line longer than 120 .{color:green}+1{color} the patch does adds/modifies 3 testcase(s) {color:green}+1 RAT{color} .{color:green}+1{color} the patch does not seem to introduce new RAT warnings {color:green}+1 JAVADOC{color} .{color:green}+1{color} the patch does not seem to introduce new Javadoc warnings {color:green}+1 COMPILE{color} .{color:green}+1{color} HEAD compiles .{color:green}+1{color} patch compiles .{color:green}+1{color} the patch does not seem to introduce new javac warnings {color:green}+1 FINDBUGS{color} .{color:green}+1{color} the patch does not seem to introduce new Findbugs warnings {color:green}+1 TESTS{color} .Tests run: 888 {color:green}+1 DISTRO{color} .{color:green}+1{color} distro tarball builds with the patch {color:green}*+1 Overall result, good!, no -1s*{color} The full output of the test-patch run is available at . https://builds.apache.org/job/bookkeeper-trunk-precommit-build/557/ Turn readonly back to writable if spaces are reclaimed. --- Key: BOOKKEEPER-661 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-661 Project: Bookkeeper Issue Type: Improvement Components: bookkeeper-server Reporter: Sijie Guo Assignee: Sijie Guo Fix For: 4.3.0 Attachments: BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661_662.diff should be able to turn a bookie from readonly back to writable if the spaces are reclaimed. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (BOOKKEEPER-661) Turn readonly back to writable if spaces are reclaimed.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-661?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13873307#comment-13873307 ] Ivan Kelly commented on BOOKKEEPER-661: --- Ooops, seems I resolved by mistake. Will take another look. Turn readonly back to writable if spaces are reclaimed. --- Key: BOOKKEEPER-661 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-661 Project: Bookkeeper Issue Type: Improvement Components: bookkeeper-server Reporter: Sijie Guo Assignee: Sijie Guo Fix For: 4.3.0 Attachments: BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661_662.diff should be able to turn a bookie from readonly back to writable if the spaces are reclaimed. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (BOOKKEEPER-661) Turn readonly back to writable if spaces are reclaimed.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-661?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13873383#comment-13873383 ] Ivan Kelly commented on BOOKKEEPER-661: --- The logic in checkRegNodeAndWaitExpire and registerBookie is a bit off. If the node exists and is owned by the current session, the zk.create() in registerBookie shouldn't be called. Turn readonly back to writable if spaces are reclaimed. --- Key: BOOKKEEPER-661 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-661 Project: Bookkeeper Issue Type: Improvement Components: bookkeeper-server Reporter: Sijie Guo Assignee: Sijie Guo Fix For: 4.3.0 Attachments: BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661_662.diff should be able to turn a bookie from readonly back to writable if the spaces are reclaimed. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (BOOKKEEPER-661) Turn readonly back to writable if spaces are reclaimed.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-661?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13873526#comment-13873526 ] Sijie Guo commented on BOOKKEEPER-661: -- my bad. I was thinking returning boolean to not create znode. but it slipped from my mind. will address that. Turn readonly back to writable if spaces are reclaimed. --- Key: BOOKKEEPER-661 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-661 Project: Bookkeeper Issue Type: Improvement Components: bookkeeper-server Reporter: Sijie Guo Assignee: Sijie Guo Fix For: 4.3.0 Attachments: BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661_662.diff should be able to turn a bookie from readonly back to writable if the spaces are reclaimed. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (BOOKKEEPER-661) Turn readonly back to writable if spaces are reclaimed.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-661?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13873664#comment-13873664 ] Rakesh R commented on BOOKKEEPER-661: - Thanks Sijie for the patch. Adding few comments, please see: # 'regPath' is missing in logging, please pass args to it. {code} LOG.error(ZK exception checking and wait ephemeral znode {} expired : , ke); LOG.error(Interrupted checking and wait ephemeral znode {} expired : , ie); {code} # Please add (timeout=6) for the test case. {code} +public void testBookieShouldTurnWritableFromReadOnly() throws Exception { {code} Also, since you are touching it would be great if you can include timeout for other testcases in ReadOnlyBookieTest class. Somehow we have missed this class when doing the BOOKKEEPER-523 imprv activity. # It is good practise to have fail statement in tests when we are expecting an exception. Please use like the below one. {code} +try { +ledger.addEntry(data.getBytes()); Assert.fail(Should throw exception as not enough bookies to write entry!); +} catch (BKException.BKNotEnoughBookiesException e) { +// Expected +} +try { +bkc.createLedger(2, 2, DigestType.MAC, .getBytes()); Assert.fail(Should throw exception as not enough bookies to create ledger!); +} catch (BKException bke) { +// Expected. +} {code} Turn readonly back to writable if spaces are reclaimed. --- Key: BOOKKEEPER-661 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-661 Project: Bookkeeper Issue Type: Improvement Components: bookkeeper-server Reporter: Sijie Guo Assignee: Sijie Guo Fix For: 4.3.0 Attachments: BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661_662.diff should be able to turn a bookie from readonly back to writable if the spaces are reclaimed. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (BOOKKEEPER-661) Turn readonly back to writable if spaces are reclaimed.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-661?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13872266#comment-13872266 ] Rakesh R commented on BOOKKEEPER-661: - bq.On any unrelated note, why do we register readonly bookies in zookeeper. I don't see it used anywhere on the client? Rakesh R, perhaps you remember? Hi Ivan, As I recollect, readonly bookie information is used in the following cases: # Rereplication : Auditor will be using all the bookies(available + r-o bookies) to see failures. # There is a logic to close channels to dead bookies. This readonly bookieId is used to make it alive and excluding from deadbookie list. # Also, exposed shell command to know the r-o bookies Turn readonly back to writable if spaces are reclaimed. --- Key: BOOKKEEPER-661 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-661 Project: Bookkeeper Issue Type: Improvement Components: bookkeeper-server Reporter: Sijie Guo Assignee: Sijie Guo Fix For: 4.3.0 Attachments: BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661_662.diff should be able to turn a bookie from readonly back to writable if the spaces are reclaimed. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (BOOKKEEPER-661) Turn readonly back to writable if spaces are reclaimed.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-661?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13872498#comment-13872498 ] Sijie Guo commented on BOOKKEEPER-661: -- {quote} It would be better to modify registerBookie() to get check if the Stat.getEphemeralOwner == current zk session id. In this case, registerBookie can just return. I think this is safer. {quote} checking session id is the safer way. will do that. {quote} Do we need diskWritable() diskJustWritable()? They seem to be handled the same. {quote} they are related to DiskWarnThreshold and DiskSpaceThreshold. we handle different action in gc. when disk is just writable, we need to force gc. Turn readonly back to writable if spaces are reclaimed. --- Key: BOOKKEEPER-661 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-661 Project: Bookkeeper Issue Type: Improvement Components: bookkeeper-server Reporter: Sijie Guo Assignee: Sijie Guo Fix For: 4.3.0 Attachments: BOOKKEEPER-661.diff, BOOKKEEPER-661.diff, BOOKKEEPER-661_662.diff should be able to turn a bookie from readonly back to writable if the spaces are reclaimed. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (BOOKKEEPER-661) Turn readonly back to writable if spaces are reclaimed.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-661?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13859829#comment-13859829 ] Rakesh R commented on BOOKKEEPER-661: - Good work Sijie:) I've gone through 'back to writable logic' and please see the below comments. I'll look into GC part separately and will add my comments to BOOKKEEPER-662. # small typo : should be 'serve' instead of 'server' {code} LOG.info(Transitioning Bookie to Writable mode and will server read/write requests.); {code} # small typo : should be 'Failed' instead of 'Faile' {code} LOG.warn(Faile to delete bookie readonly state in zookeeper : , e); {code} # Wrongly deleting the readonly path, bookie is creating ephemral path as follows. Please use the same for deletion {code} this.bookieRegistrationPath + BookKeeperConstants.READONLY + / + getMyId() {code} # Since it is ignoring the ZK KeeperException when deleting, its good to handle KeeperException.NodeExistsException during transitionToReadOnlyMode() and here it will triggerBookieShutdown() {code} // clear the readonly state // ... } catch (KeeperException e) { // if we failed when deleting the readonly flag in zookeeper, it is OK since client would // already see the bookie in writable list. so just log the exception LOG.warn(Faile to delete bookie readonly state in zookeeper : , e); return; } {code} -Rakesh Turn readonly back to writable if spaces are reclaimed. --- Key: BOOKKEEPER-661 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-661 Project: Bookkeeper Issue Type: Improvement Components: bookkeeper-server Reporter: Sijie Guo Assignee: Sijie Guo Fix For: 4.3.0 Attachments: BOOKKEEPER-661.diff should be able to turn a bookie from readonly back to writable if the spaces are reclaimed. -- This message was sent by Atlassian JIRA (v6.1.5#6160)
[jira] [Commented] (BOOKKEEPER-661) Turn readonly back to writable if spaces are reclaimed.
[ https://issues.apache.org/jira/browse/BOOKKEEPER-661?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13843032#comment-13843032 ] Hadoop QA commented on BOOKKEEPER-661: -- Testing JIRA BOOKKEEPER-661 Patch [BOOKKEEPER-661.diff|https://issues.apache.org/jira/secure/attachment/12617797/BOOKKEEPER-661.diff] downloaded at Mon Dec 9 09:38:31 UTC 2013 {color:green}+1 PATCH_APPLIES{color} {color:green}+1 CLEAN{color} {color:green}+1 RAW_PATCH_ANALYSIS{color} .{color:green}+1{color} the patch does not introduce any @author tags .{color:green}+1{color} the patch does not introduce any tabs .{color:green}+1{color} the patch does not introduce any trailing spaces .{color:green}+1{color} the patch does not introduce any line longer than 120 .{color:green}+1{color} the patch does adds/modifies 5 testcase(s) {color:green}+1 RAT{color} .{color:green}+1{color} the patch does not seem to introduce new RAT warnings {color:green}+1 JAVADOC{color} .{color:green}+1{color} the patch does not seem to introduce new Javadoc warnings {color:green}+1 COMPILE{color} .{color:green}+1{color} HEAD compiles .{color:green}+1{color} patch compiles .{color:green}+1{color} the patch does not seem to introduce new javac warnings {color:green}+1 FINDBUGS{color} .{color:green}+1{color} the patch does not seem to introduce new Findbugs warnings {color:green}+1 TESTS{color} .Tests run: 887 {color:green}+1 DISTRO{color} .{color:green}+1{color} distro tarball builds with the patch {color:green}*+1 Overall result, good!, no -1s*{color} The full output of the test-patch run is available at . https://builds.apache.org/job/bookkeeper-trunk-precommit-build/552/ Turn readonly back to writable if spaces are reclaimed. --- Key: BOOKKEEPER-661 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-661 Project: Bookkeeper Issue Type: Improvement Components: bookkeeper-server Reporter: Sijie Guo Assignee: Sijie Guo Fix For: 4.3.0 Attachments: BOOKKEEPER-661.diff should be able to turn a bookie from readonly back to writable if the spaces are reclaimed. -- This message was sent by Atlassian JIRA (v6.1.4#6159)