Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add and check indexDirs in Cookie meta #3372

Merged
merged 1 commit into from
Jul 1, 2022

Conversation

StevenLuMT
Copy link
Member

@StevenLuMT StevenLuMT commented Jun 28, 2022

Descriptions of the changes in this PR:

Motivation

planning for index dir: mail talking
stage 2: add and check indexDirs in cookie meta

Changes

  1. add indexDirs in cookie meta
  2. check indexDirs in cookie meta
  3. add a testcase for cookie

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work
some questions

  1. how do we deal with rollbacks of versions ?
  2. is this change backward compatible ?

@StevenLuMT
Copy link
Member Author

great work some questions

  1. how do we deal with rollbacks of versions ?
  2. is this change backward compatible ?

@eolivelli
good question,the code is backward compatible :

  1. when set the existed config ServerConfiguration#setIndexDirName, the new cookie will work
  2. when not set this config, the old testcase(CookieTest) do the regression testing
  3. if old bookeeper node want to upgrade this cookie, later I will add a new pr for adding a funtion in cookie_generate

@eolivelli
Copy link
Contributor

there should be already a bookkeeper shell command to rewrite the cookie

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@StevenLuMT
Copy link
Member Author

3. adding a funtion in cookie_generate

yes , it's cookie_generate(class name is GenerateCookieCommand.java), I will add a param for --index-dirs
another new pr @eolivelli

@eolivelli eolivelli merged commit dc09182 into apache:master Jul 1, 2022
@hangc0276 hangc0276 added this to the 4.16.0 milestone Jul 25, 2022
gaozhangmin pushed a commit to gaozhangmin/bookkeeper that referenced this pull request Feb 23, 2023
Co-authored-by: lushiji <lushiji@didiglobal.com>
(cherry picked from commit dc09182)
Ghatage pushed a commit to sijie/bookkeeper that referenced this pull request Jul 12, 2024
Co-authored-by: lushiji <lushiji@didiglobal.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants