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

parallel-benchmark: Store data in SQLite for long runs #29956

Merged
merged 2 commits into from
Oct 16, 2024

Conversation

def-
Copy link
Contributor

@def- def- commented Oct 11, 2024

Seems not to cause a performance difference, but lacks some features currently

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

@def- def- changed the title WIP: SQLite store WIP: SQLite store for parallel-benchmark Oct 14, 2024
@def- def- force-pushed the pr-sqlite-store branch 3 times, most recently from 6914a16 to 618a779 Compare October 15, 2024 13:28
@def- def- changed the title WIP: SQLite store for parallel-benchmark parallel-benchmark: Store data in SQLite for long runs Oct 15, 2024
@def- def- force-pushed the pr-sqlite-store branch 4 times, most recently from 3f9fccf to d4ee745 Compare October 15, 2024 21:45
Seems not to cause a performance difference, but lacks some features currently
@def- def- marked this pull request as ready for review October 15, 2024 21:50
@def- def- requested a review from a team as a code owner October 15, 2024 21:50
Copy link
Contributor

@nrainer-materialize nrainer-materialize left a comment

Choose a reason for hiding this comment

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

What's the motivation for this change?

test/parallel-benchmark/mzcompose.py Outdated Show resolved Hide resolved
@def-
Copy link
Contributor Author

def- commented Oct 16, 2024

The QA Canary job runs parallel-benchmark for 24 hours, it would require a beefy machine to keep all the results in memory, so I'd rather have a small machine and store the results on disk.

@def- def- enabled auto-merge October 16, 2024 07:21
@nrainer-materialize
Copy link
Contributor

The QA Canary job runs parallel-benchmark for 24 hours, it would require a beefy machine to keep all the results in memory, so I'd rather have a small machine and store the results on disk.

Makes sense! I was not aware that we are collecting that much data.

(m.scenario, action),
)
self.queries, self.qps, self.max, self.min, self.avg = cursor.fetchone()
# TODO: Rest
Copy link
Contributor

Choose a reason for hiding this comment

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

There is an open TODO. Do you plan to address this within this PR?

Note that I cancelled auto-merge. Feel free to proceed, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, I left that for future me.

@def- def- merged commit a0f04e1 into MaterializeInc:main Oct 16, 2024
82 checks passed
@def- def- deleted the pr-sqlite-store branch October 16, 2024 12:22
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