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.

Sign up for GitHub

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

Jump to bottom

[issue 8337][Worker] Move initialize dlog namespace metadata to bin/pulsar #8781

Merged
merged 2 commits into from
Jan 12, 2021
Merged

[issue 8337][Worker] Move initialize dlog namespace metadata to bin/pulsar #8781

merged 2 commits into from
Jan 12, 2021

Conversation

nodece
Copy link
Member

@nodece nodece commented Dec 2, 2020 •
edited
Loading

Fixes #8337

Motivation

Currently, the starting function worker service will initialize the distributed log namespace. when initializing the distributed log namespace, the zookeeper will be connected. A better way is to implement a tool to initialize the distributed log namespace, which is similar to ./bin/pulsar initialize-cluster-metadata.

Modifications

  • Add init distributed log namespace metadata to ./bin/pulsar initialize-cluster-metadata
  • Add flag to control whether the distributed log namespace is initialized in Function worker
  • Add flag to conf/functions_worker.yml

Doc

If you want to initialize distributed log metadata by bin/pulsar, you need to perform the following steps:

  1. Using ./bin/pulsar initialize-cluster-metadata to initialize cluster
  2. Set initializedDlogMetadata to true in functions_worker.yml

Note: All the changes are compatible.

Verifying this change

  • Make sure that the change passes the CI checks.

@nodece nodece mentioned this pull request Dec 2, 2020
Closed
sijie
sijie requested changes Dec 2, 2020
View reviewed changes
Copy link
Member

@sijie sijie left a comment

Choose a reason for hiding this comment

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

Overall the change looks good to me. I left two comments. PTAL.

pulsar-functions/worker/src/main/java/org/apache/pulsar/functions/worker/Worker.java Outdated Show resolved Hide resolved
pulsar-broker/src/main/java/org/apache/pulsar/PulsarInitialDlogNamespaceMetadataSetup.java Outdated Show resolved Hide resolved
@sijie sijie added this to the 2.8.0 milestone Dec 2, 2020
@sijie sijie added area/cli type/feature The PR added a new feature or issue requested a new feature and removed area/broker labels Dec 2, 2020
@nodece nodece requested a review from sijie December 2, 2020 10:23
@nodece nodece force-pushed the move_init_dlog_namespace branch 5 times, most recently from dcc634c to 4f5d8d5 Compare December 7, 2020 07:07
@tuteng
Copy link
Member

tuteng commented Dec 22, 2020

ping @nodece

@nodece
Copy link
Member Author

nodece commented Jan 4, 2021 •
edited
Loading

@tuteng @sijie could you review the PR again?

@nodece nodece requested a review from sijie January 4, 2021 08:16
@tuteng
Copy link
Member

tuteng commented Jan 4, 2021

We may need to consider adding some documentation on how to use this command @jennifer88huang

@tuteng tuteng added the doc-required Your PR changes impact docs and you will update later. label Jan 4, 2021
@Jennifer88huang-zz
Copy link
Contributor

@tuteng thank you for your reminder.
@nodece you can add related doc content in this PR or in a separate PR, or you can also ask a writer to add it. If you need any support, feel free to let us know. Thank you.

eolivelli
eolivelli approved these changes Jan 4, 2021
View reviewed changes
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

+1

@nodece
Add initialize dlog namespace metadata to bin/pulsar
c341217 
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@nodece
Copy link
Member Author

nodece commented Jan 5, 2021 •
edited
Loading

@tuteng @jennifer88huang thanks, I did update at #8781 (comment), but I'm not sure what to add document to the official website.

@nodece
Copy link
Member Author

nodece commented Jan 5, 2021

@tuteng @jennifer88huang once pr is merged, I will make a new PR for this doc.

@tuteng
Copy link
Member

tuteng commented Jan 5, 2021

/pulsarbot run-failure-checks

@Jennifer88huang-zz
Copy link
Contributor

@tuteng @jennifer88huang once pr is merged, I will make a new PR for this doc.

That's great, thank you.

@Jennifer88huang-zz
Copy link
Contributor

/pulsarbot run-failure-checks

1 similar comment
@tuteng
Copy link
Member

tuteng commented Jan 7, 2021

/pulsarbot run-failure-checks

@tuteng
Copy link
Member

tuteng commented Jan 9, 2021

/pulsarbot run-failure-checks

1 similar comment
@tuteng
Copy link
Member

tuteng commented Jan 11, 2021

/pulsarbot run-failure-checks

@nodece
Copy link
Member Author

nodece commented Jan 12, 2021

@tuteng @sijie tests has beed passed. could you merge this to master?

@sijie sijie merged commit aca4cd7 into apache:master Jan 12, 2021
@nodece
Copy link
Member Author

nodece commented Jan 12, 2021

@jennifer88huang I made a PR for document: #9185.

@sijie sijie mentioned this pull request Jan 12, 2021
Merged
@codelipenghui
Copy link
Contributor

Hi @tuteng I noticed this PR is based on #8560, #8560 has some big changes so it will not contains in the 2.7.x release. Do we need to cherry-pick this PR to branch-2.7?

@tuteng
Copy link
Member

tuteng commented Jan 14, 2021 •
edited
Loading

@codelipenghui Thanks, move this pr to 2.8.0, delete label release/2.7.1

@sijia-w
Copy link
Contributor

sijia-w commented Mar 10, 2021

@nodece Just a soft reminder. We still need to add doc for the update in this pr.

@tuteng @jennifer88huang once pr is merged, I will make a new PR for this doc.

@Anonymitaet
Copy link
Member

@sijia-w does the author already add docs in #9185?

@sijia-w
Copy link
Contributor

sijia-w commented Mar 15, 2021

@Anonymitaet It seems so. Thanks for reminding me.

@Anonymitaet Anonymitaet removed the doc-required Your PR changes impact docs and you will update later. label Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sijie sijie sijie approved these changes

@eolivelli eolivelli eolivelli approved these changes

@zymap zymap zymap approved these changes

Assignees

@nodece nodece

Labels
area/cli type/feature The PR added a new feature or issue requested a new feature
Projects
None yet
Milestone
2.8.0
Development

Successfully merging this pull request may close these issues.

Create a tool to initialize dlog namespace
9 participants
@nodece @tuteng @Jennifer88huang-zz @codelipenghui @sijia-w @Anonymitaet @sijie @eolivelli @zymap