You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@bookkeeper.apache.org by Ivan Kelly <iv...@apache.org> on 2012/02/15 15:08:46 UTC

Review Request: BOOKKEEPER-172 Upgrade framework for filesystem layouts

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

Review request for bookkeeper.


Summary
-------

Part of BOOKKEEPER-163. This improvement creates a framework to allow administrators to upgrade the filesystem layout of previous bookkeeper versions to be usable by the most recent software. The upgrade processes are currently empty until BOOKKEEPER-163 adds something.


This addresses bug BOOKKEEPER-172.
    https://issues.apache.org/jira/browse/BOOKKEEPER-172


Diffs
-----

  bookkeeper-server/bin/bookkeeper b0629bd 
  bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java PRE-CREATION 
  bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/3910/diff


Testing
-------


Thanks,

Ivan

Re: Review Request: BOOKKEEPER-172 Upgrade framework for filesystem layouts

Posted by Ivan Kelly <iv...@apache.org>.


> On 2012-02-21 10:06:21, Sijie Guo wrote:
> > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java, line 68
> > <https://reviews.apache.org/r/3910/diff/1/?file=75146#file75146line68>
> >
> >     should it be a constant define?

No, unless we define it as LAYOUT_VERSION_1 which kinda defeats the purpose. I never what this number to be changed, so having a magic number here is fine in my opinion. 


- Ivan


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


On 2012-02-15 14:08:46, Ivan Kelly wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3910/
> -----------------------------------------------------------
> 
> (Updated 2012-02-15 14:08:46)
> 
> 
> Review request for bookkeeper.
> 
> 
> Summary
> -------
> 
> Part of BOOKKEEPER-163. This improvement creates a framework to allow administrators to upgrade the filesystem layout of previous bookkeeper versions to be usable by the most recent software. The upgrade processes are currently empty until BOOKKEEPER-163 adds something.
> 
> 
> This addresses bug BOOKKEEPER-172.
>     https://issues.apache.org/jira/browse/BOOKKEEPER-172
> 
> 
> Diffs
> -----
> 
>   bookkeeper-server/bin/bookkeeper b0629bd 
>   bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java PRE-CREATION 
>   bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3910/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ivan
> 
>

Re: Review Request: BOOKKEEPER-172 Upgrade framework for filesystem layouts

Posted by Sijie Guo <gu...@gmail.com>.

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



bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java
<https://reviews.apache.org/r/3910/#comment11437>

    should it be a constant define?


- Sijie


On 2012-02-15 14:08:46, Ivan Kelly wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3910/
> -----------------------------------------------------------
> 
> (Updated 2012-02-15 14:08:46)
> 
> 
> Review request for bookkeeper.
> 
> 
> Summary
> -------
> 
> Part of BOOKKEEPER-163. This improvement creates a framework to allow administrators to upgrade the filesystem layout of previous bookkeeper versions to be usable by the most recent software. The upgrade processes are currently empty until BOOKKEEPER-163 adds something.
> 
> 
> This addresses bug BOOKKEEPER-172.
>     https://issues.apache.org/jira/browse/BOOKKEEPER-172
> 
> 
> Diffs
> -----
> 
>   bookkeeper-server/bin/bookkeeper b0629bd 
>   bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java PRE-CREATION 
>   bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3910/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ivan
> 
>

Re: Review Request: BOOKKEEPER-172 Upgrade framework for filesystem layouts

Posted by Ivan Kelly <iv...@apache.org>.


> On 2012-02-21 06:26:00, fpj wrote:
> > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java, line 22
> > <https://reviews.apache.org/r/3910/diff/1/?file=75146#file75146line22>
> >
> >     Shouldn't this go into bookkeeper.tools?

No, this accesses bookie internals. If this were put in bookkeeper.tools, then bookie internals would have to be made public. 


> On 2012-02-21 06:26:00, fpj wrote:
> > bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java, line 115
> > <https://reviews.apache.org/r/3910/diff/1/?file=75146#file75146line115>
> >
> >     Sounds like we need documentation for this. It is ok if we are adding in BOOKKEEPER-163.

Documentation for the tool in general, or documentation for this object? This is the same configuration as the bookie server uses.


- Ivan


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


On 2012-02-15 14:08:46, Ivan Kelly wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3910/
> -----------------------------------------------------------
> 
> (Updated 2012-02-15 14:08:46)
> 
> 
> Review request for bookkeeper.
> 
> 
> Summary
> -------
> 
> Part of BOOKKEEPER-163. This improvement creates a framework to allow administrators to upgrade the filesystem layout of previous bookkeeper versions to be usable by the most recent software. The upgrade processes are currently empty until BOOKKEEPER-163 adds something.
> 
> 
> This addresses bug BOOKKEEPER-172.
>     https://issues.apache.org/jira/browse/BOOKKEEPER-172
> 
> 
> Diffs
> -----
> 
>   bookkeeper-server/bin/bookkeeper b0629bd 
>   bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java PRE-CREATION 
>   bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3910/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ivan
> 
>

Re: Review Request: BOOKKEEPER-172 Upgrade framework for filesystem layouts

Posted by fp...@apache.org.

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


Looks good, I have a couple of minor comments. 


bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java
<https://reviews.apache.org/r/3910/#comment11422>

    Shouldn't this go into bookkeeper.tools?



bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java
<https://reviews.apache.org/r/3910/#comment11433>

    Sounds like we need documentation for this. It is ok if we are adding in BOOKKEEPER-163.


- fpj


On 2012-02-15 14:08:46, Ivan Kelly wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3910/
> -----------------------------------------------------------
> 
> (Updated 2012-02-15 14:08:46)
> 
> 
> Review request for bookkeeper.
> 
> 
> Summary
> -------
> 
> Part of BOOKKEEPER-163. This improvement creates a framework to allow administrators to upgrade the filesystem layout of previous bookkeeper versions to be usable by the most recent software. The upgrade processes are currently empty until BOOKKEEPER-163 adds something.
> 
> 
> This addresses bug BOOKKEEPER-172.
>     https://issues.apache.org/jira/browse/BOOKKEEPER-172
> 
> 
> Diffs
> -----
> 
>   bookkeeper-server/bin/bookkeeper b0629bd 
>   bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java PRE-CREATION 
>   bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3910/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ivan
> 
>