Skip to content

Druid automated quickstart: zookeeper in service list#13550

Merged
vogievetsky merged 4 commits intoapache:masterfrom
findingrish:druid-quickstart-add-zk-to-service-list
Dec 12, 2022
Merged

Druid automated quickstart: zookeeper in service list#13550
vogievetsky merged 4 commits intoapache:masterfrom
findingrish:druid-quickstart-add-zk-to-service-list

Conversation

@findingrish
Copy link
Copy Markdown
Contributor

@findingrish findingrish commented Dec 12, 2022

Fixes #13543.

Description

This change removes zk arg and instead allows passing zookeeper in the --s argument for the startup script.
e.g. bin/start-druid -s broker,router,historical,coordinator-overlord,middleManager,zookeeper

Key changed/added classes in this PR
  • start-druid-main.py

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

Comment thread examples/bin/start-druid-main.py Outdated
-c=conf/druid/single-server/custom \\
--zk
Starts broker, router and zookeeper.
broker and router config is read from given root directory.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
broker and router config is read from given root directory.
Configs for broker and router are read from the specified root directory.

'{broker, router, middleManager, historical, coordinator-overlord, indexer}. \n'
'{broker, router, middleManager, historical, coordinator-overlord, indexer, zk}. \n'
'If the argument is not given, broker, router, middleManager, historical, coordinator-overlord \n'
'and zookeeper is started. e.g. -s=broker,historical')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: the full word was better, I guess

Comment thread examples/bin/start-druid-main.py Outdated
MIDDLE_MANAGER = "middleManager"
TASKS = "tasks"
INDEXER = "indexer"
ZK = "zk"
Copy link
Copy Markdown
Contributor

@kfaraz kfaraz Dec 12, 2022

Choose a reason for hiding this comment

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

I think it might be better to just use the full name zookeeper since we are not doing an abbreviation for the other services either.

@kfaraz kfaraz added this to the 25.0 milestone Dec 12, 2022
Copy link
Copy Markdown
Contributor

@kfaraz kfaraz left a comment

Choose a reason for hiding this comment

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

Minor comments, otherwise LGTM.

@findingrish findingrish changed the title Druid automated quickstart: zk in service list Druid automated quickstart: zookeeper in service list Dec 12, 2022
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.

Add zk to the possible service list in the new bin/start-druid script

3 participants