Skip to content

MINOR: Update ZooKeeper upgrade notes - #7818

Closed
omkreddy wants to merge 4 commits into
apache:trunkfrom
omkreddy:zk-note
Closed

MINOR: Update ZooKeeper upgrade notes#7818
omkreddy wants to merge 4 commits into
apache:trunkfrom
omkreddy:zk-note

Conversation

@omkreddy

Copy link
Copy Markdown
Contributor

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@omkreddy
omkreddy requested a review from ijuma December 11, 2019 14:43
@ijuma

ijuma commented Dec 11, 2019

Copy link
Copy Markdown
Member

cc @edenhill

Comment thread docs/upgrade.html Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What are "testing upgrades"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@edenhill Thanks for the review. I have updated the statement. Please take a look.

Comment thread docs/upgrade.html Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • "Make sure to update your local zookeeper.properties file with "enable.adminserver=false" (or whatever the property is) if you wish to disable the AdminServer."

@edenhill edenhill left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread docs/upgrade.html Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we change "ZooKeeper has been upgraded from 3.4.14 to 3.5.5" to "ZooKeeper has been upgraded from 3.4.14 to 3.5.6"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ijuma Thanks for the review. Updated the docs. Please take one more look.

@omkreddy
omkreddy force-pushed the zk-note branch 2 times, most recently from 5879ac2 to edcfb6d Compare December 13, 2019 05:21
@omkreddy
omkreddy requested a review from cmccabe December 13, 2019 05:21
Comment thread docs/upgrade.html Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure why we would ever recommend setting snapshot.trust.empty, when we know that it is buggy. We should explicitly tell them not to do this, then link to the ZK JIRA explaining why not. Then we should explain what the right solution is (downloading the blank snapshot file)

@omkreddy omkreddy Dec 14, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We have observed data loss only in standalone cluster upgrades. So I have recommended snapshot.trust.empty flag for multi-node cluster clusters. I just updated the note to avoid unnecessary confusion.

@cmccabe @ijuma Can you take a look please. I would like to include above note to 2.4.0 upgrade docs..

@omkreddy
omkreddy force-pushed the zk-note branch 3 times, most recently from 52eba37 to 6ec8223 Compare December 14, 2019 17:55
@cmccabe

cmccabe commented Dec 19, 2019

Copy link
Copy Markdown
Contributor

LGTM

@omkreddy omkreddy closed this in 96edabb Dec 19, 2019
@omkreddy

Copy link
Copy Markdown
Contributor Author

Thanks for the reviews. Merging to trunk and 2.4.

omkreddy added a commit that referenced this pull request Dec 19, 2019
Author: Manikumar Reddy <manikumar.reddy@gmail.com>

Reviewers: Ismael Juma <ismael@juma.me.uk>, Magnus Edenhill <magnus@edenhill.se>, Colin Patrick McCabe <cmccabe@apache.org>

Closes #7818 from omkreddy/zk-note
qq619618919 pushed a commit to qq619618919/kafka that referenced this pull request May 12, 2020
Author: Manikumar Reddy <manikumar.reddy@gmail.com>

Reviewers: Ismael Juma <ismael@juma.me.uk>, Magnus Edenhill <magnus@edenhill.se>, Colin Patrick McCabe <cmccabe@apache.org>

Closes apache#7818 from omkreddy/zk-note
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.

4 participants