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.

Sign up for GitHub

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

Jump to bottom

Fix counter for memtable updates #2986

Closed
wants to merge 1 commit into from
Closed

Fix counter for memtable updates #2986

wants to merge 1 commit into from

Conversation

miasantreble
Copy link
Contributor

Right now in PutCFImpl we always increment NUMBER_KEYS_UPDATED counter for both in-place update or insertion. This PR fixes this by using the correct counter for either case.

facebook-github-bot
facebook-github-bot reviewed Oct 10, 2017
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@miasantreble has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

yiwu-arbug
yiwu-arbug reviewed Oct 10, 2017
View reviewed changes
Copy link
Contributor

@yiwu-arbug yiwu-arbug left a comment

Choose a reason for hiding this comment

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

How about also move the other NUMBER_KEYS_WRITTEN usage from WriteBatch to MemTable::Add()?

@facebook-github-bot
Copy link
Contributor

@miasantreble has updated the pull request. View: changes, changes since last import

facebook-github-bot
facebook-github-bot reviewed Oct 10, 2017
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@miasantreble has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@miasantreble has updated the pull request.

facebook-github-bot
facebook-github-bot reviewed Oct 11, 2017
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@miasantreble has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@miasantreble
Copy link
Contributor Author

looks like NUMBER_KEYS_WRITTEN was updated in DBImpl::WriteImpl and DBImpl::PipelinedWriteImpl together with other related counters, so it makes more sense to keep it that way and focus this PR on NUMBER_KEYS_UPDATED instead

facebook-github-bot
facebook-github-bot reviewed Oct 11, 2017
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@miasantreble is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@miasantreble miasantreble deleted the fix-stats-memtable-update branch October 11, 2017 04:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@yiwu-arbug yiwu-arbug yiwu-arbug approved these changes

@ajkr ajkr Awaiting requested review from ajkr

Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@miasantreble @facebook-github-bot @yiwu-arbug