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

transform: start loading VMs after logs are started #17965

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

rockwotj
Copy link
Contributor

Let's make sure we start the log manager before we start loading Wasm
VMs. Probably not going to cause an issue, but just to be safe.

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.3.x
  • v23.2.x

Release Notes

  • none

@rockwotj rockwotj requested a review from oleiman April 19, 2024 15:41
@github-actions github-actions bot added area/redpanda area/wasm WASM Data Transforms labels Apr 19, 2024
@rockwotj rockwotj self-assigned this Apr 19, 2024
oleiman
oleiman previously approved these changes Apr 19, 2024
Copy link
Member

@oleiman oleiman left a comment

Choose a reason for hiding this comment

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

LGTM. Does this mean you need to rejigger the order in service::stop?

Let's make sure we start the log manager before we start loading Wasm
VMs. Probably not going to cause an issue, but just to be safe.

Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
@rockwotj
Copy link
Contributor Author

LGTM. Does this mean you need to rejigger the order in service::stop?

Good question, the order should be reversed between the two (like destructor order), so I've flipped the startup to match the stop method, as the stop method looks more correct for me

@rockwotj rockwotj requested a review from oleiman April 19, 2024 16:33
Copy link
Member

@oleiman oleiman left a comment

Choose a reason for hiding this comment

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

👍

@rockwotj
Copy link
Contributor Author

CI failed due to infra issues

@rockwotj rockwotj merged commit 5a212bb into redpanda-data:dev Apr 19, 2024
13 of 17 checks passed
@rockwotj rockwotj deleted the start-wasm-logger-first branch April 19, 2024 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/redpanda area/wasm WASM Data Transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants