> On 2012-03-09 06:38:44, Sijie Guo wrote:
> > bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieTest.java,
> >  line 151
> > <https://reviews.apache.org/r/3926/diff/1/?file=75432#file75432line151>
> >
> >     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?

I did have this in the back of my mind when writing this, but it seems an extra 
feature to me and would bloat the scope a bit much. Could we put this in an new 
JIRA?


> On 2012-03-09 06:38:44, Sijie Guo wrote:
> > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java,
> >  line 173
> > <https://reviews.apache.org/r/3926/diff/1/?file=75428#file75428line173>
> >
> >     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.

Very good point, will do this.


> On 2012-03-09 06:38:44, Sijie Guo wrote:
> > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java,
> >  line 63
> > <https://reviews.apache.org/r/3926/diff/1/?file=75428#file75428line63>
> >
> >     it seems that you missed that file 'lastId' and 'lastMark'

Ah, well spotted, will add them.


> On 2012-03-09 06:38:44, Sijie Guo wrote:
> > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Cookie.java, 
> > line 135
> > <https://reviews.apache.org/r/3926/diff/1/?file=75426#file75426line135>
> >
> >     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.

Fixed


- Ivan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3926/#review5766
-----------------------------------------------------------


On 2012-02-16 16:15:58, Ivan Kelly wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3926/
> -----------------------------------------------------------
> 
> (Updated 2012-02-16 16:15:58)
> 
> 
> Review request for bookkeeper.
> 
> 
> Summary
> -------
> 
> 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 addresses bug BOOKKEEPER-163.
>     https://issues.apache.org/jira/browse/BOOKKEEPER-163
> 
> 
> Diffs
> -----
> 
>   bookkeeper-server/pom.xml 601104f 
>   bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java 
> 2fb7c6c 
>   
> bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieException.java
>  1a5b313 
>   bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Cookie.java 
> PRE-CREATION 
>   
> bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java 
> aca66e6 
>   
> bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java
>  PRE-CREATION 
>   
> bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerCache.java 
> 3e96d46 
>   
> bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java
>  2df0ed0 
>   
> bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieLayoutVersionTest.java
>  10f9538 
>   
> bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieTest.java 
> PRE-CREATION 
>   
> bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/EntryLogTest.java
>  f661e90 
>   
> bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java 
> PRE-CREATION 
>   
> bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieRecoveryTest.java
>  99258ac 
>   
> bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestFencing.java 
> 015e4e4 
>   
> bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java 
> dada67a 
>   
> bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieFailureTest.java
>  ea51118 
>   
> bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieZKExpireTest.java
>  5ed7061 
>   
> bookkeeper-server/src/test/java/org/apache/bookkeeper/test/ZooKeeperUtil.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3926/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ivan
> 
>

Reply via email to