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

Fix ever existing iterator and transaction in write benchmark #1457

Merged
merged 3 commits into from
Aug 14, 2020

Conversation

ahsanbarkati
Copy link
Contributor

@ahsanbarkati ahsanbarkati commented Aug 14, 2020

This PR adds an option to enable showing keys statistics in the write benchmark. The introduced flag is --show-keys and is disabled by default. This PR also fixes the ever existing iterators in the reportStats() function.


This change is Reviewable

@ahsanbarkati ahsanbarkati changed the title Fix ever existing iterator and transaction write benchmark Fix ever existing iterator and transaction in write benchmark Aug 14, 2020
Copy link
Contributor

@jarifibrahim jarifibrahim left a comment

Choose a reason for hiding this comment

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

Minor comments

badger/cmd/write_bench.go Outdated Show resolved Hide resolved
badger/cmd/write_bench.go Outdated Show resolved Hide resolved
Copy link
Contributor

@jarifibrahim jarifibrahim left a comment

Choose a reason for hiding this comment

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

lgtm

@ahsanbarkati ahsanbarkati reopened this Aug 14, 2020
@ahsanbarkati ahsanbarkati merged commit d31355d into master Aug 14, 2020
@ahsanbarkati ahsanbarkati deleted the ahsan/iterators branch August 14, 2020 15:21
jarifibrahim pushed a commit that referenced this pull request Oct 2, 2020
91a80ef (Add support collecting stats (sst count, vlog count, file sizes, move key count, valid keys etc) introduced count of various types of keys in the report (log) of the benchmark write tool. For counting various keys, iterators were used in the function `reportStats()` and the keys were counted periodically. For closing the iterators `defer it.close()` was used, which never closed iterators until `reportStats()` returned. This caused the iterators to accumulate and hence resulting in excessive memory usage. 

Now, this key count is made optional by the introduction of a new option `--show-keys`, which is disabled by default. The statistics is collected in a separate function and the iterators are properly closed after each iteration cycle. Also the transaction is properly discarded after each run.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants