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

Enable Function Workers to use exclusive producer to write to internal topics #9275

Merged

Conversation

jerrypeng
Copy link
Contributor

Motivation

To maintain correctness, we need to use exclusive producer to write to the internal topics Pulsar Functions to manage metadata / state.

@jerrypeng jerrypeng added this to the 2.8.0 milestone Jan 22, 2021
@jerrypeng jerrypeng self-assigned this Jan 22, 2021
.supplier(() -> {
try {
Producer<byte[]> producer = client.newProducer().topic(topic)
.accessMode(ProducerAccessMode.Exclusive)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to use WaitForExclusive here? So that the producer instance will stay pending until it becomes the leader?

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 don't think it is a good idea for "liveness" is we block forever which may cause deadlocks. While blocking, the worker might be not even be the leader anymore. I think we should keep retrying and periodically check if we are still the leader.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @jerrypeng

try {
Producer<byte[]> producer = client.newProducer().topic(topic)
.accessMode(ProducerAccessMode.Exclusive)
.enableBatching(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you remember if there was any reason for not using batching here?

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 do not. We could enable it but I wouldn't expect much benefit as we are not exactly optimizing for metadata operation throughput. For debugging it is a little easier without batching as you will know exactly how many messages are in the internal topics.

@jerrypeng jerrypeng changed the title Enable Function Workers to use exclusive producer to write to internal topics WIP Enable Function Workers to use exclusive producer to write to internal topics Jan 25, 2021
@eolivelli
Copy link
Contributor

What happens if the broker is still on an old version that does not support ExclusiveProducer mode ?

@jerrypeng
Copy link
Contributor Author

@eolivelli

What happens if the broker is still on an old version that does not support ExclusiveProducer mode ?

If the broker hasn't been updated and does not support exclusive producing mode, the client can still connect and produce but exclusivity is not guaranteed

@eolivelli
Copy link
Contributor

Thanks. Makes sense

@jerrypeng jerrypeng changed the title WIP Enable Function Workers to use exclusive producer to write to internal topics Enable Function Workers to use exclusive producer to write to internal topics Feb 16, 2021
@jerrypeng jerrypeng changed the title Enable Function Workers to use exclusive producer to write to internal topics WIP Enable Function Workers to use exclusive producer to write to internal topics Feb 16, 2021
@jerrypeng jerrypeng force-pushed the add_exclusive_producer_to_functions branch from 17cd361 to 2fc028a Compare February 16, 2021 22:03
@jerrypeng jerrypeng changed the title WIP Enable Function Workers to use exclusive producer to write to internal topics Enable Function Workers to use exclusive producer to write to internal topics Feb 16, 2021
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.

4 participants