Skip to content

Conversation

petrosagg
Copy link
Contributor

Motivation

The intent is to remove these load generators completely but at the moment we use them in our internal test suite quite extensively. Putting them behind a feature flag ensures we won't get any external users depending on them.

Tips for reviewer

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.

@petrosagg petrosagg requested review from a team as code owners October 13, 2025 21:39
@petrosagg petrosagg requested a review from ggevay October 13, 2025 21:39
@petrosagg petrosagg force-pushed the feature-flag-loadgen branch 3 times, most recently from f4cd022 to 4a64c73 Compare October 13, 2025 22:49
'CREATE SOURCE' ('IF NOT EXISTS')? src_name
('IN CLUSTER' cluster_name)?
'FROM LOAD GENERATOR' ('AUCTION' | 'COUNTER' | 'MARKETING' | 'TPCH' | 'KEY VALUE')
'FROM LOAD GENERATOR' ('AUCTION' | 'MARKETING' | 'TPCH' | 'KEY VALUE')
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you want to also remove KEY VALUE?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also the relevant options from the syntax diagram (e.g., VALUE SIZE)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's an internal-only type that is also not used in many places. I'll probably delete it in a subsequent PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, you mean that I forgot to do it. Yes, you're absolutely right. Will remove now

DROP MATERIALIZED VIEW IF EXISTS counter_sum;
DROP SOURCE IF EXISTS counter;
DROP MATERIALIZED VIEW IF EXISTS amount_sum;
DROP SOURCE IF EXISTS auction;
Copy link
Contributor

Choose a reason for hiding this comment

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

CASCADE for dropping auction?

We will eventually phase out these load generators so for now make sure
no one relies on them except our tests.

Signed-off-by: Petros Angelatos <[email protected]>
@petrosagg petrosagg force-pushed the feature-flag-loadgen branch from 4a64c73 to 85df858 Compare October 14, 2025 19:48
@petrosagg petrosagg enabled auto-merge October 14, 2025 19:48
Copy link
Contributor

@kay-kim kay-kim left a comment

Choose a reason for hiding this comment

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

Wrong source name ... but once those are fixed, lgtm.

DROP MATERIALIZED VIEW IF EXISTS counter_sum;
DROP SOURCE IF EXISTS counter;
DROP MATERIALIZED VIEW IF EXISTS amount_sum;
DROP SOURCE IF EXISTS amount;
Copy link
Contributor

Choose a reason for hiding this comment

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

Heh ... DROP SOURCE IF EXISTS auction CASCADE;

DROP MATERIALIZED VIEW IF EXISTS counter_sum;
DROP SOURCE IF EXISTS counter;
DROP MATERIALIZED VIEW IF EXISTS ammount_sum;
DROP SOURCE IF EXISTS amount;
Copy link
Contributor

Choose a reason for hiding this comment

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

same. DROP SOURCE IF EXISTS auction CASCADE;

Copy link
Contributor

@kay-kim kay-kim left a comment

Choose a reason for hiding this comment

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

Actually, saw that this was in an auto-merge ... so, changed review status.

@petrosagg petrosagg disabled auto-merge October 14, 2025 20:03
@petrosagg petrosagg force-pushed the feature-flag-loadgen branch from 85df858 to 63b90d8 Compare October 14, 2025 20:06
@petrosagg
Copy link
Contributor Author

@kay-kim sorry about those typos! It should be good to go now

Copy link
Contributor

@kay-kim kay-kim left a comment

Choose a reason for hiding this comment

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

lgtm 🎉 Thank you!!!!

@petrosagg petrosagg merged commit c233527 into MaterializeInc:main Oct 14, 2025
129 checks passed
@petrosagg petrosagg deleted the feature-flag-loadgen branch October 14, 2025 23:30
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.

3 participants