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

Rename Fill to Write Benchmark Tool #875

Merged
merged 14 commits into from
Jun 18, 2019
Merged

Conversation

ashish-goswami
Copy link
Contributor

@ashish-goswami ashish-goswami commented Jun 18, 2019

Previous PR(#859) for above changes was raised against a different branch. Raising it against master again. I have merged latest changes from master in this branch.


This change is Reviewable

* Single routine for printing stats
* Get Keys via stream framework
* Add sample-size and keys-only size
* Fix code to have exactly sample no of keys
* Wrap returned errors
* Change function names
* Add comments
* Optimise stream to read only keys while sampling
* Addres review comments
Idea is to have hierarchy for benchmark commands.
* Add license
* Change default go routine to 16
* Make read only optional
* Make loading mode optional
* Change reader and printer routine names
* Modify KeyToList func of StreamWriter
* Shuffle keys before sampling
* Change file name fill to write_bench.go
* Add code to print writestats
* Remove check "G404: Use of weak random number generator (math/rand
instead of crypto/rand) (from gosec)" from gas linter
@ashish-goswami ashish-goswami requested a review from a team June 18, 2019 07:49
Copy link
Member

@mangalaman93 mangalaman93 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @ashish-goswami, @jarifibrahim, and @manishrjain)


.golangci.yml, line 26 at r1 (raw file):

    - linters:
        - gosec
      text: "G404: "

newline missing


badger/cmd/write_bench.go, line 31 at r1 (raw file):

	"github.com/dgraph-io/badger/pb"
	"github.com/dgraph-io/badger/y"
	humanize "github.com/dustin/go-humanize"

separate imports

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:

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @ashish-goswami, @jarifibrahim, and @manishrjain)

@ashish-goswami ashish-goswami merged commit 6d3b67d into master Jun 18, 2019
@ashish-goswami ashish-goswami deleted the ashish/fill-rename branch June 18, 2019 11:11
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.

3 participants