-
Notifications
You must be signed in to change notification settings - Fork 594
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
test: add Kafka consumer-group, add-partition tests (inline style) #16244
Conversation
Signed-off-by: xxchan <xxchan22f@gmail.com>
@@ -0,0 +1,108 @@ | |||
# This file contains commands used by the tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commands added here are actually not used in the slt
file (although in theory they can). They are now mainly helper commands to run manually.
e.g., risedev rpk
is similar to risedev psql
. It sets RPK_BROKERS
env var. So it's helpful for local testing.
commit b3c8e4e Author: xxchan <xxchan22f@gmail.com> Date: Sat Apr 13 02:39:13 2024 +0800 fix Signed-off-by: xxchan <xxchan22f@gmail.com> commit a575c3a Author: xxchan <xxchan22f@gmail.com> Date: Sat Apr 13 02:38:15 2024 +0800 fix Signed-off-by: xxchan <xxchan22f@gmail.com> commit df9f905 Author: xxchan <xxchan22f@gmail.com> Date: Sat Apr 13 02:26:00 2024 +0800 update Signed-off-by: xxchan <xxchan22f@gmail.com> commit b8859ca Author: xxchan <xxchan22f@gmail.com> Date: Sat Apr 13 02:25:14 2024 +0800 fix Signed-off-by: xxchan <xxchan22f@gmail.com> commit f001e31 Author: xxchan <xxchan22f@gmail.com> Date: Sat Apr 13 02:23:06 2024 +0800 revert risedev Signed-off-by: xxchan <xxchan22f@gmail.com> commit 33aa178 Merge: 530eb86 c98dfd5 Author: xxchan <xxchan22f@gmail.com> Date: Sat Apr 13 02:22:13 2024 +0800 Merge branch 'xxchan/latin-tyrannosaurus' into xxchan/source-test commit 530eb86 Merge: 0b6f74c cf221e3 Author: xxchan <xxchan22f@gmail.com> Date: Sat Apr 13 02:21:42 2024 +0800 Merge branch 'xxchan/latin-tyrannosaurus' into xxchan/source-test Signed-off-by: xxchan <xxchan22f@gmail.com> commit 0b6f74c Author: xxchan <xxchan22f@gmail.com> Date: Sat Apr 13 02:11:40 2024 +0800 sleep more? Signed-off-by: xxchan <xxchan22f@gmail.com> commit 47c2c61 Author: xxchan <xxchan22f@gmail.com> Date: Fri Apr 12 22:30:09 2024 +0800 sleep more Signed-off-by: xxchan <xxchan22f@gmail.com> commit 1853adf Author: xxchan <xxchan22f@gmail.com> Date: Fri Apr 12 21:59:31 2024 +0800 fix Signed-off-by: xxchan <xxchan22f@gmail.com> commit 8711cf5 Merge: f1c0185 4f53f89 Author: xxchan <xxchan22f@gmail.com> Date: Fri Apr 12 17:52:38 2024 +0800 Merge remote-tracking branch 'origin/main' into xxchan/source-test Signed-off-by: xxchan <xxchan22f@gmail.com> commit f1c0185 Author: xxchan <xxchan22f@gmail.com> Date: Fri Apr 12 17:52:12 2024 +0800 fix Signed-off-by: xxchan <xxchan22f@gmail.com> commit e9e2dc4 Author: xxchan <xxchan22f@gmail.com> Date: Fri Apr 12 16:49:20 2024 +0800 install rpk Signed-off-by: xxchan <xxchan22f@gmail.com> commit e2f2a8e Author: xxchan <xxchan22f@gmail.com> Date: Fri Apr 12 15:16:44 2024 +0800 f Signed-off-by: xxchan <xxchan22f@gmail.com> commit bae6495 Merge: 58de398 4ac029c Author: xxchan <xxchan22f@gmail.com> Date: Fri Apr 12 15:11:54 2024 +0800 Merge remote-tracking branch 'origin/main' into xxchan/source-test commit 58de398 Author: xxchan <xxchan22f@gmail.com> Date: Fri Apr 12 15:11:49 2024 +0800 fix Signed-off-by: xxchan <xxchan22f@gmail.com> commit 9ba9728 Author: xxchan <xxchan22f@gmail.com> Date: Thu Apr 11 17:45:24 2024 +0800 fix Signed-off-by: xxchan <xxchan22f@gmail.com> commit d8a2489 Author: xxchan <xxchan22f@gmail.com> Date: Thu Apr 11 17:18:38 2024 +0800 fix Signed-off-by: xxchan <xxchan22f@gmail.com> commit c4b4b16 Author: xxchan <xxchan22f@gmail.com> Date: Thu Apr 11 17:16:25 2024 +0800 fix Signed-off-by: xxchan <xxchan22f@gmail.com> commit 6dddbf3 Author: xxchan <xxchan22f@gmail.com> Date: Thu Apr 11 17:15:00 2024 +0800 rename new to inline Signed-off-by: xxchan <xxchan22f@gmail.com> commit cf50f5e Author: xxchan <xxchan22f@gmail.com> Date: Thu Apr 11 17:04:54 2024 +0800 bump Signed-off-by: xxchan <xxchan22f@gmail.com> commit 9ca9d55 Author: xxchan <xxchan22f@gmail.com> Date: Thu Apr 11 16:00:07 2024 +0800 support user-managed kafka in risedev Signed-off-by: xxchan <xxchan22f@gmail.com> commit d4d405d Author: xxchan <xxchan22f@gmail.com> Date: Thu Apr 11 15:27:20 2024 +0800 update Signed-off-by: xxchan <xxchan22f@gmail.com> commit 6ed6cfc Author: xxchan <xxchan22f@gmail.com> Date: Thu Apr 11 10:21:25 2024 +0800 update commit f7a3dd5 Merge: 8b6c482 254ad0c Author: xxchan <xxchan22f@gmail.com> Date: Thu Apr 11 09:45:50 2024 +0800 Merge remote-tracking branch 'origin/main' into xxchan/source-test commit 8b6c482 Author: xxchan <xxchan22f@gmail.com> Date: Tue Apr 9 14:12:37 2024 +0800 update commit f280603 Author: xxchan <xxchan22f@gmail.com> Date: Fri Apr 5 23:14:30 2024 +0800 add new source tests commit 0e3ace1 Author: xxchan <xxchan22f@gmail.com> Date: Fri Apr 5 23:13:05 2024 +0800 revert unrelated change commit a3f3409 Author: xxchan <xxchan22f@gmail.com> Date: Fri Apr 5 17:58:48 2024 +0800 fix commit 0a2ded0 Author: xxchan <xxchan22f@gmail.com> Date: Fri Apr 5 17:27:45 2024 +0800 fix commit 8daa64d Author: xxchan <xxchan22f@gmail.com> Date: Fri Apr 5 17:19:38 2024 +0800 debug commit 8628c1f Author: xxchan <xxchan22f@gmail.com> Date: Fri Apr 5 17:15:32 2024 +0800 fix commit c8bde20 Author: xxchan <xxchan22f@gmail.com> Date: Fri Apr 5 17:00:09 2024 +0800 ci: install risedev to ci image Signed-off-by: xxchan <xxchan22f@gmail.com>
b3c8e4e
to
143d569
Compare
0ab4742
to
6f1d622
Compare
Signed-off-by: xxchan <xxchan22f@gmail.com>
|
||
sleep 2s | ||
|
||
# Verify that RisingWave's Kafka consumer works independently from the console consumers subscribing to the same group. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test case is intended for #16111
control substitution on | ||
|
||
system ok | ||
rpk topic create test_add_partition -p 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test case is for #15994 (incomplete yet, still need some more cases)
Signed-off-by: xxchan <xxchan22f@gmail.com>
sleep 5s | ||
|
||
system ok | ||
./e2e_test/source_inline/kafka/consumer_group.mjs --mv mv list-members |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using js here is just a personal choice. It doesn't matter much. Can also use sh
, py
...
Here a script is needed and plain bash command is not enough because we need to first query fragment id, and then filter consumer groups with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we automatically download/install zx
for developers running tests locally, like through cargo make
? Or, include this in the developer documentation.
Signed-off-by: xxchan <xxchan22f@gmail.com>
} | ||
ServiceConfig::Kafka(c) => { | ||
let brokers = format!("{}:{}", c.address, c.port); | ||
writeln!(env, r#"RISEDEV_KAFKA_BOOTSTRAP_SERVERS="{brokers}""#,).unwrap(); | ||
writeln!(env, r#"RISEDEV_KAFKA_WITH_OPTIONS_COMMON="connector='kafka',properties.bootstrap.server='{brokers}'""#).unwrap(); | ||
writeln!(env, r#"RPK_BROKERS="{brokers}""#).unwrap(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a core change
- use: kafka | ||
user-managed: true | ||
address: message_queue | ||
port: 29092 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a core change
sleep 5s | ||
|
||
system ok | ||
./e2e_test/source_inline/kafka/consumer_group.mjs --mv mv list-members |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we automatically download/install zx
for developers running tests locally, like through cargo make
? Or, include this in the developer documentation.
|
||
# The lag for batch query's group is 0, and each MV parition's group is 2 (1 of 3 consumed). | ||
system ok | ||
./e2e_test/source_inline/kafka/consumer_group.mjs --mv mv list-lags |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this requires that current working directory is at the project root. Can we organize these utilities together and add them to the PATH
somehow? 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this requires that current working directory is at the project root. Can we organize these utilities together and add them to the PATH somehow? 😄
I also thought about this, but not very sure. It can indeed make the script look nicer. (There's also another benefit: we can make the scripts a "project", and do more powerful stuff like adding dependencies to it.)
risedev
always runs in the project root. We assume the slt is run byrisedev slt
, not only for the path issue, but also env vars, so even if we changed it, we still need to userisedev slt
.- As @stdrc mentioned test: "inline" style connector e2e tests #12451 (comment), (also like DocSlt) maybe orginaze everything together is clearer. Adding more indirection also increases burden to find the scripts. (first indirection is put command in script, second indirection is put the script somewhere else and in PATH). Although running the tests doesn't require multiple steps now, but maybe not good for reading/writing.
So I'm a little hesitant about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess: if the script is ad-hoc (only used for one test), it's better to put next to the test file. If it is general (and as we have more such scripts), it's better to put together. It's kind of like "built-in" commands for the test driver. 😄
For this PR, I prefer not to change it as the benefits haven't shown.
rpk topic create test_consumer_group -p 3 | ||
|
||
system ok | ||
cat <<EOF | rpk topic produce test_consumer_group -f "%p %v\\n" -p 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also extract this pipeline into a single command?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you suggest an improvement? I feel the command is simple enough. Adding a wrapper just makes it less clear (unnecessary indirection)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For instance, if we are going to adopt the same "%p %v\n"
format in several places...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will also use other formats, especially "%k %v\n" I guess
Signed-off-by: xxchan <xxchan22f@gmail.com>
Signed-off-by: xxchan <xxchan22f@gmail.com>
Signed-off-by: xxchan <xxchan22f@gmail.com>
Signed-off-by: xxchan <xxchan22f@gmail.com>
e309ef7
to
3757c4f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥰
Signed-off-by: xxchan <xxchan22f@gmail.com>
Signed-off-by: xxchan <xxchan22f@gmail.com>
Signed-off-by: xxchan <xxchan22f@gmail.com>
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
To think about changes in this PR, we can assume we implement this in a step to step manner:
e2e_test/source/cdc_inline/alter/postgres_alter.slt
) to write such test for Kafka. Now we explicitly use environment variables to set kafka brokers. Before running tests (CI/local), we set env vars.js
to write some more complex scripts.risedev-env
, so that we don't need to set envvars locally (when we userisedev slt
.user-managed
Kafka inrisedev.yml
, so that we don't need to set envvars in CI. Now everything is unified.The spirit of this PR, and more details about what's changed ("Example: Integrate Kafka" section) are described in #12451 (comment)
Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.