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

feat: uses internal topics prefix for shared runtimes #8607

Merged
merged 3 commits into from
Jan 20, 2022
Merged

feat: uses internal topics prefix for shared runtimes #8607

merged 3 commits into from
Jan 20, 2022

Conversation

wcarlson5
Copy link
Contributor

@wcarlson5 wcarlson5 commented Jan 18, 2022

Without shared runtimes: _confluent-ksql-default_query_CSAS_CART_EVENT_PRODUCT_1-Join-repartition
shared runtimes without change: _confluent-ksql-default_query_-1-CSAS_CART_EVENT_PRODUCT_1-Join-repartition
shared runtimes with change: _confluent-ksql-default_query-CSAS_CART_EVENT_PRODUCT_1-Join-repartition

Will remove the runtime Id from internal topics so that we will be able to use topic in a different application with later work

Testing done

Describe the testing strategy. Unit and integration tests are expected for any behavior changes.

Reviewer checklist

  • Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
  • Ensure relevant issues are linked (description should include text like "Fixes #")

@wcarlson5 wcarlson5 requested a review from a team as a code owner January 18, 2022 22:47
Copy link
Contributor

@ableegoldman ableegoldman left a comment

Choose a reason for hiding this comment

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

LGTTM!

@wcarlson5 wcarlson5 merged commit 44d85b2 into confluentinc:master Jan 20, 2022
@wcarlson5 wcarlson5 deleted the use_topic_prefix branch January 20, 2022 15:11
@guozhangwang
Copy link
Contributor

@wcarlson5 should we change the logic in KafkaTopicClientImpl too, where applicationId is used as the default prefix?

@wcarlson5
Copy link
Contributor Author

wcarlson5 commented Jan 21, 2022

@guozhangwang do you mean when deleting internal topics?

I think we do need to but it might require a bit more thought because we will need to change how that is done

@guozhangwang
Copy link
Contributor

I think we do need to but it might require a bit more thought because we will need to change how that is done

Yes I was thinking about the purging of internal topics upon terminating the query indeed. Please let me know what procedure changes we'd need to consider.

@wcarlson5
Copy link
Contributor Author

@guozhangwang Sophie reminded me I already made that change. I just need to adapt the new thing to use the new prefix instead of the application like you mentioned. I am going to leave KafkaTopicClientImpl's logic the same just call it with the prefix instead of app ID

@guozhangwang
Copy link
Contributor

Okay, thanks!

@wcarlson5
Copy link
Contributor Author

This caused an issue for running old queries on the same server as queries in the new runtime. The fix can be found here #9470

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