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

Temporarily disable coproc on ci #4557

Merged
merged 5 commits into from
May 6, 2022

Conversation

Rob Blafford added 5 commits May 4, 2022 15:08
- Many obscure wasm test failures can be traced to a root issue of a
previous wasm engine still existing and listening on port 43189.

- When this occurs only some of the deployed wasm engines will be
started and the test output will confusingly show not enough data was
read from redpanda.
- Killing the wasm_engine should be a part of the clean_node() routine

- Fixes: redpanda-data#4038
- These tests all attempt to remove a materialized topic while the
coprocessor is still running.

- However due to the initial design of the system, coproc will attempt
to recreate the topic and re-populate it up until the previous high
watermark. If this was not performed there would be an inconsistency
between the coprocessors defined metadata and random commands sent to the
cluster by the user.

- The tests have been beneficial in understanding that this type of
concurrent delete can occur without any crashes.

- If a user wants to truly delete a materialized topic he/she must
shutdown the coprocessor first.

- Fixes: redpanda-data#3384
- Disabling temporarily due to a difficult to debug memory leak detected
by ASAN on rare occassions.

- Marking issue for tracking here: redpanda-data#4053

@ok_to_fail # https://github.com/redpanda-data/redpanda/issues/3745
@cluster(num_nodes=4)
def verify_materialized_topics_test(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm concerned about the gap left in test coverage after this commit is merged. Should we document somewhere that we need a WasmDeleteTopics test in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left the explanation in the commit message, is there somewhere else better we could put this? Maybe a link in the source?

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair point.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think when we re-pick up the WASM workstream, we will want to look at what all tests are required. That said, I wonder if we leave such tests...commented out rather than delete?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I specifically deleted it as stated in the commit because it was working as expected. Deleting materialized topics while coprocessors are still processing inputs may re-create said topics

@NyaliaLui NyaliaLui self-requested a review May 6, 2022 15:07
@graphcareful graphcareful merged commit 6b92046 into redpanda-data:dev May 6, 2022
Comment on lines +16 to +19
- tests/wasm_topics_test.py
- tests/wasm_identity_test.py
- tests/wasm_partition_movement_test.py
- tests/wasm_redpanda_failure_recovery_test.py
Copy link
Member

Choose a reason for hiding this comment

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

why aren't we using @ignore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to clutter ci output with more coproc stuff

@ajfabbri
Copy link
Contributor

ajfabbri commented Jun 3, 2022

Attempting to backport this to clear up CI issues affecting 22.1.4 backport PRs, e.g. #5012

@ajfabbri
Copy link
Contributor

ajfabbri commented Jun 3, 2022

/backport v22.1.x

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.

5 participants