[
https://issues.apache.org/jira/browse/BOOKKEEPER-163?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13225885#comment-13225885
]
[email protected] commented on BOOKKEEPER-163:
----------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3926/#review5766
-----------------------------------------------------------
most of the patch is good beside some small issues. I commented them inline.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Cookie.java
<https://reviews.apache.org/r/3926/#comment12563>
if more than two bookie server try to create COOKIE_PATH at the same time,
it would fail. it would be better to catch NodeExists Exception.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java
<https://reviews.apache.org/r/3926/#comment12565>
it seems that you missed that file 'lastId' and 'lastMark'
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java
<https://reviews.apache.org/r/3926/#comment12564>
it would be better to copy data to a temp directory, such as upgrading,
then rename the upgrading directory to current.
otherwise, if the upgrading is broken after created current directory, but
user restarts bookie server with this upgrade-failed directory, bookie server
would treat it as a new environment.
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieTest.java
<https://reviews.apache.org/r/3926/#comment12566>
for a new disk (new directory) added case, it would not cause data missing.
could we provide a tool to upgrade a bookie's cookie when adding a new
directory?
- Sijie
On 2012-02-16 16:15:58, Ivan Kelly wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/3926/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-02-16 16:15:58)
bq.
bq.
bq. Review request for bookkeeper.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. bookkeeper client treats NoSuchLedgerException as valid response when
reading last confirmed. If NoSuchLedgerException is caused due to an empty
directory in following cases, it is an incorrect response.
bq.
bq. 1) A disk is replaced or ledger index is removed by a sloppy admin.
bq. 2) A disk is not mounted when a bookie machine is restarted.
bq.
bq. We need a mechanism to prevent such incorrect responses.
bq.
bq. Ivan suggested to generate a instance key for each bookie and write it
into the ledger directories. If a directory doesn't have the key, and other
directories do, then it shouldn't start. This would also resolve the issue that
someone starting a new bookie with the same IP as a bookie which has previously
died.
bq.
bq.
bq. This addresses bug BOOKKEEPER-163.
bq. https://issues.apache.org/jira/browse/BOOKKEEPER-163
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. bookkeeper-server/pom.xml 601104f
bq. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
2fb7c6c
bq.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieException.java
1a5b313
bq. bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Cookie.java
PRE-CREATION
bq.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
aca66e6
bq.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java
PRE-CREATION
bq.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCache.java
3e96d46
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java
2df0ed0
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieLayoutVersionTest.java
10f9538
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieTest.java
PRE-CREATION
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/EntryLogTest.java
f661e90
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java
PRE-CREATION
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieRecoveryTest.java
99258ac
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestFencing.java
015e4e4
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java
dada67a
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieFailureTest.java
ea51118
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieZKExpireTest.java
5ed7061
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/ZooKeeperUtil.java
PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/3926/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Ivan
bq.
bq.
> Prevent incorrect NoSuchLedgerException for readLastConfirmed.
> --------------------------------------------------------------
>
> Key: BOOKKEEPER-163
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-163
> Project: Bookkeeper
> Issue Type: Bug
> Components: bookkeeper-client, bookkeeper-server
> Affects Versions: 4.0.0
> Reporter: Sijie Guo
> Assignee: Ivan Kelly
> Fix For: 4.1.0
>
> Attachments: BOOKKEEPER-163.diff
>
>
> bookkeeper client treats NoSuchLedgerException as valid response when reading
> last confirmed. If NoSuchLedgerException is caused due to an empty directory
> in following cases, it is an incorrect response.
> 1) A disk is replaced or ledger index is removed by a sloppy admin.
> 2) A disk is not mounted when a bookie machine is restarted.
> We need a mechanism to prevent such incorrect responses.
> Ivan suggested to generate a instance key for each bookie and write it into
> the ledger directories. If a directory doesn't have the key, and other
> directories do, then it shouldn't start. This would also resolve the issue
> that someone starting a new bookie with the same IP as a bookie which has
> previously died.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
