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

Issue 116: Use IP address as Bookie ID #117

Merged
merged 1 commit into from
Jan 14, 2019
Merged

Conversation

adrianmo
Copy link
Contributor

Change log description

  • Change Bookie ID from using the pod's hostname to the IP address.

Purpose of the change

Related to #116

This is a temporary workaround to fix BookKeeper failover scenarios until Pravega's BookKeeper version is updated to 4.7 (WiP: pravega/pravega#3192).

How to test

BookieFailoverTest should pass.


Signed-off-by: Adrián Moreno adrian@morenomartinez.com

Signed-off-by: Adrián Moreno <adrian@morenomartinez.com>
@adrianmo adrianmo self-assigned this Jan 11, 2019
@adrianmo adrianmo requested a review from fpj January 11, 2019 14:19
@adrianmo
Copy link
Contributor Author

@shrids can you please test the BookieFailoverTest using a custom operator image build based on this branch?

@adrianmo adrianmo requested a review from shrids January 11, 2019 15:26
@adrianmo
Copy link
Contributor Author

I've build an image containing the changes in this PR and pushed it to adrianmo/pravega-operator:issue-116 to make it easier to validate the fix.

@RaulGracia
Copy link

RaulGracia commented Jan 14, 2019

I tested this change in a system test build and is taking effect:

2019-01-14 11:36:47,813 4392906 [bookkeeper-io-1] DEBUG o.a.b.proto.PerChannelBookieClient - Successfully wrote request for adding entry: 30022 ledger-id: 12 bookie: 10.200.34.91/10.200.34.91:3181 entry length: 219
2019-01-14 11:36:47,813 4392906 [bookkeeper-io-0] DEBUG o.a.b.proto.PerChannelBookieClient - Successfully wrote request for adding entry: 30022 ledger-id: 12 bookie: 10.200.85.71/10.200.85.71:3181 entry length: 219
2019-01-14 11:36:47,813 4392906 [bookkeeper-io-0] DEBUG o.a.b.proto.PerChannelBookieClient - Successfully wrote request for adding entry: 30022 ledger-id: 12 bookie: 10.200.61.77/10.200.61.77:3181 entry length: 219

Now IPs are used instead of hostnames. BookieFailoverTest is still failing, but the problem is the MinimumBookkeeperReplicas = 3 constant defined, which makes it impossible to downscale the number of Bookies to 2 and the test timeouts.

Copy link

@RaulGracia RaulGracia left a comment

Choose a reason for hiding this comment

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

The change takes effect in a system test build.

@adrianmo adrianmo merged commit 3c7ea47 into master Jan 14, 2019
@adrianmo adrianmo deleted the issue-116-bk-id-ip branch January 14, 2019 19:13
Copy link

@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.

Makes sense

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.

3 participants