Skip to content

feat: Add batch size bytes as additional option#582

Merged
kodiakhq[bot] merged 5 commits intomainfrom
batch-size
Jan 9, 2023
Merged

feat: Add batch size bytes as additional option#582
kodiakhq[bot] merged 5 commits intomainfrom
batch-size

Conversation

@hermanschaaf
Copy link
Copy Markdown
Contributor

This adds batchSizeBytes as an additional option to limit batches not only by number of records, but also by number of bytes.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 6, 2023

⏱️ Benchmark results

  • DefaultConcurrencyDFS-2 resources/s: 10,027
  • DefaultConcurrencyRoundRobin-2 resources/s: 11,016
  • Glob-2 ns/op: 184.1
  • TablesWithChildrenDFS-2 resources/s: 30,149
  • TablesWithChildrenRoundRobin-2 resources/s: 29,141
  • TablesWithRateLimitingDFS-2 resources/s: 28.4
  • TablesWithRateLimitingRoundRobin-2 resources/s: 837
  • BufferedScanner-2 ns/op: 11.36
  • LogReader-2 ns/op: 34.81

}
return
}
if len(resources) == p.spec.BatchSize || sizeBytes+r.Size() > p.spec.BatchSizeBytes {
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.

Using zero as the default value shortcut breaks my heart every time. Maybe allow negative values (erm, -1) to mean "unlimited"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry, not sure I understood the comment... Was it meant to be a comment on this line? If the batch size limit is 0, the SetDefaults call will ensure it's updated to the default batch size. So there will always be some limit, but you can of course set it to 1PB if you don't want it to affect anything in practice

Copy link
Copy Markdown
Contributor

@yevgenypats yevgenypats left a comment

Choose a reason for hiding this comment

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

LGTM!

@kodiakhq kodiakhq bot merged commit bdd76e0 into main Jan 9, 2023
@kodiakhq kodiakhq bot deleted the batch-size branch January 9, 2023 09:03
kodiakhq bot pushed a commit that referenced this pull request Jan 9, 2023
🤖 I have created a release *beep* *boop*
---


## [1.23.0](v1.22.0...v1.23.0) (2023-01-09)


### Features

* Add batch size bytes as additional option ([#582](#582)) ([bdd76e0](bdd76e0))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
kodiakhq bot pushed a commit to cloudquery/cloudquery that referenced this pull request Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants