Skip to content

Issue #2305: Bookies should not be allowed to come up without a cookie in metadata store.#2306

Merged
sijie merged 1 commit intoapache:masterfrom
Ghatage:disableBootup-Wo-Cookie
May 19, 2020
Merged

Issue #2305: Bookies should not be allowed to come up without a cookie in metadata store.#2306
sijie merged 1 commit intoapache:masterfrom
Ghatage:disableBootup-Wo-Cookie

Conversation

@Ghatage
Copy link
Copy Markdown
Contributor

@Ghatage Ghatage commented Apr 12, 2020

Motivation

Currently we allow bookies to come up even if they don't have a cookie in the metadata store.
The cookie is an identity of a registered bookie and a bookie should not be allowed to come up without it.

Changes

Update handling of bookie boot up. Add test.
This scenario is rarely expected but can happen in case of corruption or errant deletion of znodes for cookies

@Ghatage
Copy link
Copy Markdown
Contributor Author

Ghatage commented Apr 12, 2020

Fix for #2305.
@eolivelli

Copy link
Copy Markdown
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

@eolivelli
Copy link
Copy Markdown
Contributor

Please check CI

@Ghatage
Copy link
Copy Markdown
Contributor Author

Ghatage commented Apr 13, 2020

Hi @eolivelli,
Thanks for the review, I fixed the errors.
Now it seems like a Github Actions error in the Integration tests.

[ERROR] The command '/bin/sh -c /opt/bookkeeper/scripts/install-python-client.sh' returned a non-zero code: 1
[ERROR] The command '/bin/sh -c /opt/bookkeeper/scripts/install-python-client.sh' returned a non-zero code: 1
[ERROR] Failed to execute goal com.spotify:dockerfile-maven-plugin:1.4.13:build (default) on project current-version-image: Could not build image: The command '/bin/sh -c /opt/bookkeeper/scripts/install-python-client.sh' returned a non-zero code: 1 -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <args> -rf :current-version-image
##[error]Process completed with exit code 1.

Currently we allow bookies to come up even if they don't have a cookie in the metadata store.
The cookie is an identity of a registered bookie and a bookie should not be allowed to come up without it.

Update handling of bookie boot up. Add test.
This scenario is rarely expeced but can happen in case of corruption or errant deletion of znodes for cookies
@Ghatage Ghatage force-pushed the disableBootup-Wo-Cookie branch from ed0d832 to 5a90d73 Compare April 21, 2020 20:48
Copy link
Copy Markdown
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

My concern here is that currently bookie is relying on this scenario for valid cases.

For example, the cookie is first written on local disk and then on ZK. If there's a failure in between, the bookie will currently be able to recover on next restart.

After this change, wouldn't this require manual intervention to fix?

@merlimat merlimat requested a review from ivankelly April 21, 2020 23:59
@Ghatage
Copy link
Copy Markdown
Contributor Author

Ghatage commented Apr 22, 2020

Hi @merlimat!

Yes you are right. If there is a failure in between writing the cookie to the disk and then to ZK, the bookie won't be able to restart as it will now throw an InvalidCookieException.

My concern here is that currently bookie is relying on this scenario for valid cases.

If there is no cookie in ZK, any operations which involve reading the cookie from ZK, like decommissioning a bookie via decommission command or recovering a bookie via recover command will throw NPE / fail

@Ghatage
Copy link
Copy Markdown
Contributor Author

Ghatage commented Apr 28, 2020

@ivankelly Any comments?

@eolivelli
Copy link
Copy Markdown
Contributor

@ivankelly @merlimat @jiazhai @sijie PTAL

@sijie sijie added this to the 4.11.0 milestone May 19, 2020
@sijie sijie merged commit a7ca7cf into apache:master May 19, 2020
Ghatage added a commit to Ghatage/bookkeeper that referenced this pull request Jun 19, 2020
… cookie in metadata store.

### Motivation
Currently we allow bookies to come up even if they don't have a cookie in the metadata store.
The cookie is an identity of a registered bookie and a bookie should not be allowed to come up without it.

### Changes
Update handling of bookie boot up. Add test.
This scenario is rarely expected but can happen in case of corruption or errant deletion of znodes for cookies


Reviewers: Matteo Minardi <minardi.matteo@hotmail.it>, Enrico Olivelli <eolivelli@gmail.com>, Sijie Guo <None>

This closes apache#2306 from Ghatage/disableBootup-Wo-Cookie, closes apache#2305
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants