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

Add standalone coordinator benchmarker #654

Merged
merged 6 commits into from
May 18, 2018
Merged

Conversation

arnikola
Copy link
Collaborator

No description provided.

@codecov
Copy link

codecov bot commented May 16, 2018

Codecov Report

Merging #654 into master will increase coverage by 0.51%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #654      +/-   ##
==========================================
+ Coverage   80.78%   81.29%   +0.51%     
==========================================
  Files         277      276       -1     
  Lines       24953    24599     -354     
==========================================
- Hits        20158    19998     -160     
+ Misses       3601     3406     -195     
- Partials     1194     1195       +1
Flag Coverage Δ
#coordinator 67.15% <ø> (ø) ⬆️
#db 82.25% <ø> (+0.56%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 900d4de...b60da07. Read the comment docs.

scanner := bufio.NewScanner(inFile)
for w := 0; w < workers; w++ {
for b := 0; b < batchFiles; b++ {
outFilePath := fmt.Sprintf("%s/%s%d_%d", dir, toFile, w, b)
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: You can use path.Join instead of encoding "/" in the path.

Timeseries: ts,
}
data, _ := proto.Marshal(req)
return string(snappy.Encode(nil, data))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably best to return []byte and outFile.Write(bytes) instead to avoid the large string alloc?


func main() {
if cardinality {
fmt.Println("Calculating cardinality only")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be best to use loggers everywhere? shrug.

}

start := time.Now()
log.Println("Started benchmark at:", start.Format(time.StampMilli))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe print out how many is going to be written too?

go func() {
defer wg.Done()
for batchNumber := 0; batchNumber < batches; batchNumber++ {
err = writeToCoordinator(fmt.Sprintf("%s/%s%d_%d", dataDir, dataFile, worker, batchNumber))
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Can use path.Join here too.

Copy link
Collaborator

@robskillington robskillington left a comment

Choose a reason for hiding this comment

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

LGTM once build is passing

@arnikola arnikola merged commit ea0e18b into master May 18, 2018
@arnikola arnikola deleted the arnikola/benchmarker branch May 18, 2018 21:03
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.

2 participants