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

[DPE-5109] Split Kraft mode into own service #45

Merged
merged 3 commits into from
Aug 14, 2024

Conversation

zmraul
Copy link
Contributor

@zmraul zmraul commented Aug 12, 2024

Raft mode is now supported via a new service on the snap

  • component name: kraft -> /etc/kraft/controller.properties, /var/logs/kraft/
  • service name: controller -> snap start charmed-kafka.controller

Notes

${charmed.kafka.log.level} is shared between controller and daemon modes

Copy link
Contributor

@deusebio deusebio left a comment

Choose a reason for hiding this comment

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

I'm wondering if some of the files here are actually needed or they resulted from a copy and paste of some directories. If possible, I would keep the content minimal with respect to what is needed for Kraft

Copy link
Contributor

Choose a reason for hiding this comment

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

todo I'm wondering whether the log4j for kraft need to be so similar to the one of Kafka, even featuring some zookeeper references. Also are all appender still relevant? If possible, I would prefer to make this file minimal with respect to the content

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From looking at the kafka examples, both modes (zookeeper and kraft) use the same configuration file, so I just changed references to have ${controller.logs.dir} instead of kafka logs. I can have a closer look at which specific ones are actually used, but the info here is slim

Copy link
Contributor

Choose a reason for hiding this comment

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

But they are two different processes, right? If so, honestly, I would not create any additional appender, and just log everything into a single file. We can still keep the charmed.kafka.log.* configuration, but I would make it rather minimal honestly

Copy link
Contributor

Choose a reason for hiding this comment

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

question are these beans kraft specific?

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'm not sure tbh. They are the general ones for the kafka process. I changed the name to follow a controller_[$1_$2] pattern instead of the kafka_[$1_$2] pattern.

snap/local/etc/kraft/tools-log4j.properties Outdated Show resolved Hide resolved
@zmraul zmraul merged commit 7de181b into 3/edge Aug 14, 2024
3 checks passed
@zmraul zmraul deleted the feat/dpe-5109-add-kraft-daemon branch August 14, 2024 12:32
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