Skip to content

Add to Upgrader9to10 javadoc. Closes #1922#2426

Merged
milleruntime merged 4 commits intoapache:mainfrom
milleruntime:upgrade-doc
Jan 26, 2022
Merged

Add to Upgrader9to10 javadoc. Closes #1922#2426
milleruntime merged 4 commits intoapache:mainfrom
milleruntime:upgrade-doc

Conversation

@milleruntime
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Member

@DomGarguilo DomGarguilo left a comment

Choose a reason for hiding this comment

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

LGTM

…de/Upgrader9to10.java

Co-authored-by: Dom G. <domgarguilo@apache.org>
@milleruntime
Copy link
Copy Markdown
Contributor Author

LGTM

Thanks for looking it over!

Copy link
Copy Markdown
Member

@ctubbsii ctubbsii left a comment

Choose a reason for hiding this comment

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

The class level documentation for these upgraders are probably best written as a list of expected changes that the class performs as part of its upgrade tasks. The specific methods added or implementation details or pull request references that resulted in the current code aren't useful for helping the reader of this javadoc understand what they can expect this class to do to their cluster when it upgrades it, and can either be removed, or relocated to their specific implementation methods.

@milleruntime
Copy link
Copy Markdown
Contributor Author

@ctubbsii I moved the comments to the methods in 0984633.

Copy link
Copy Markdown
Member

@ctubbsii ctubbsii left a comment

Choose a reason for hiding this comment

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

This is fine. I made one comment about some stuff that could have stayed at the class level, but it's also fine if it moves. I don't think it matters much.

Comment on lines -108 to -113
*
* Sorted recovery was updated to use RFiles instead of map files. So to prevent issues during
* tablet recovery, remove the old temporary map files and resort using RFiles. This is done in
* {@link #dropSortedMapWALFiles(ServerContext)}. For more information see the following issues:
* <a href="https://github.com/apache/accumulo/issues/2117">#2117</a> and
* <a href="https://github.com/apache/accumulo/issues/2179">#2179</a>
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.

This was probably fine to leave here, because it described the overall behavior of the Upgrader class.

@milleruntime milleruntime merged commit 912292d into apache:main Jan 26, 2022
@milleruntime milleruntime deleted the upgrade-doc branch January 26, 2022 11:37
@ctubbsii ctubbsii linked an issue Jan 26, 2022 that may be closed by this pull request
@ctubbsii ctubbsii added this to the 2.1.0 milestone Jul 12, 2024
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.

Better document Upgrader9to10

3 participants