> Yes, but it doesn't seem like a big deal. I mentioned it to identify a subtle implication of the proposed design. I think we should be intentional about what we expose to end users.
By putting this field in the protobuf, we will have more control over how users interact with this feature. > We just need to implement it > like that `ORIGINAL_MESSAGE_ID` in the retryLetterTopic. One nuanced difference between the retryLetterProducer's use of the metadata map is that the metadata fields are completely ignored by the broker and the client. They are only for future consumers of the retry letter topic. The current proposal uses the metadata map to affect broker behavior, which seems pretty different from the `ORIGIN_MESSAGE_ID` feature. > Not all cases need a new field named init_subscription when creating > producer I'm not sure this justification should prevent us from expanding the protobuf protocol: we already have several use case specific fields in the protobuf for `CommandProducer`, like for transactions, schema, and producer name [0]. > we also introduce some system message properties such as > __original_message_id in retry letter topic. I don't see this feature documented in our protocol spec [1] or in our retry letter documentation [2]. Further, it looks like this feature is only implemented in the Java and Go clients, and the Go client has a deprecated implementation because we changed the map key from `ORIGIN_MESSAGE_IDY_TIME` to `ORIGIN_MESSAGE_ID`. These are some of the risks of defining protocol elements outside of the protobuf. It is harder to implement features uniformly across our many clients and it is non trivial to propagate changes. I think it'll be easier to implement this feature consistently by expanding the protobuf. Thanks, Michael [0] https://github.com/apache/pulsar/blob/c20d55d439b67ed0c34a75b92f46674518d8bfbc/pulsar-common/src/main/proto/PulsarApi.proto#L459-L493 [1] https://pulsar.apache.org/docs/en/develop-binary-protocol/ [2] https://pulsar.apache.org/docs/en/concepts-messaging/#retry-letter-topic On Mon, Jan 24, 2022 at 1:30 AM Zike Yang <zky...@streamnative.io.invalid> wrote: > > > This is true of the DLQ producer, but by using the metadata map, won't > we be exposing this feature to end users implicitly because they could > add the field to the metadata map for other producers? (I am okay with > giving users this functionality, but I know that Matteo said in a > community meeting that this feature should be limited to the DLQ producer.) > > Yes, but it doesn't seem like a big deal. We just need to implement it > like that `ORIGINAL_MESSAGE_ID` in the retryLetterTopic. > > On Sun, Jan 23, 2022 at 2:38 PM Michael Marshall <mmarsh...@apache.org> wrote: > > > > > users will not touch the metadata of the producer of a DLQ > > > > This is true of the DLQ producer, but by using the metadata map, won't > > we be exposing this feature to end users implicitly because they could > > add the field to the metadata map for other producers? (I am okay with > > giving users this functionality, but I know that Matteo said in a > > community meeting that this feature should be limited to the DLQ producer.) > > > > > we also introduce some system message properties such as > > > __original_message_id > > > in retry letter topic. > > > > Thanks for this context. I didn't know we were already doing this. > > > > Thanks, > > Michael > > > > > > On Fri, Jan 21, 2022 at 3:35 AM Zike Yang > > <zky...@streamnative.io.invalid> wrote: > > > > > > +1 for adding init_subscription filed to the metadata of CommandProducer. > > > > > > > > > > > > On Thu, Jan 20, 2022 at 2:52 PM PengHui Li <peng...@apache.org> wrote: > > > > > > > > > What is the justification for avoiding the new protobuf field? If we > > > > add a structured field to a map of <String, String>, we are still > > > > modifying the protocol, even if we aren't modifying the protobuf. > > > > > > > > Not all cases need a new field named init_subscription when creating > > > > producer, > > > > and users will not touch the metadata of the producer of a DLQ, so I > > > > think > > > > we can use the metadata to achieve the purpose more flexibly. > > > > > > > > Yes, it modifies the protocol, but in different ways. It like the > > > > message > > > > properties, > > > > we also introduce some system message properties such as > > > > __original_message_id > > > > in retry letter topic. > > > > > > > > Penghui > > > > > > > > On Thu, Jan 20, 2022 at 2:05 PM Michael Marshall <mmarsh...@apache.org> > > > > wrote: > > > > > > > > > > I think that, good or bad, the impression that users have that the > > > > > > DLQ > > > > > > is not a "normal" topic > > > > > > > > > > Thanks for your perspective, Matteo. I still prefer my alternative > > > > > design that bypasses subscription creation, but it seems there > > > > > is insufficient interest in it, so we should move forward > > > > > discussing a DLQ specific feature and its implementation. > > > > > > > > > > > 1. Instead of modifying the current protocol, we can use producer > > > > > > metadata to carry the init subscription > > > > > > > > > > What is the justification for avoiding the new protobuf field? If we > > > > > add a structured field to a map of <String, String>, we are still > > > > > modifying the protocol, even if we aren't modifying the protobuf. > > > > > > > > > > Thanks, > > > > > Michael > > > > > > > > > > > > > > > On Tue, Jan 18, 2022 at 8:38 AM PengHui Li <peng...@apache.org> wrote: > > > > > > > > > > > > +1 for adding the DLQ_init_sub to producer metadata so that we don't > > > > > > need to introduce a new field in CommandProducer, and the new field > > > > > > looks a little confusing > > > > > > > > > > > > Thanks, > > > > > > Penghui > > > > > > > > > > > > On Mon, Jan 17, 2022 at 10:19 PM Hang Chen <chenh...@apache.org> > > > > > > wrote: > > > > > > > > > > > > > Thanks for creating this proposal Zike Yang. I have two ideas > > > > > > > about it. > > > > > > > 1. Instead of modifying the current protocol, we can use producer > > > > > > > metadata to carry the init subscription > > > > > > > 2. Please add auth for subscription creation when create topic by > > > > > > > producer, otherwise, it will be easily attacked. > > > > > > > > > > > > > > Thanks, > > > > > > > Hang > > > > > > > > > > > > > > Matteo Merli <matteo.me...@gmail.com> 于2022年1月12日周三 15:13写道： > > > > > > > > > > > > > > > > > If we want to hold that the DLQ is not a normal topic, then I > > > > > > > > > can > > > > > see > > > > > > > > > why we would have a DLQ specific feature here. > > > > > > > > > > > > > > > > I think that, good or bad, the impression that users have that > > > > > > > > the > > > > > DLQ > > > > > > > > is not a "normal" topic comes from 2 factors: > > > > > > > > 1. The experience with traditional messaging systems JMS and > > > > > > > > others > > > > > > > > where the DLQ are handled in slightly different ways, compared > > > > > > > > to > > > > > > > > other topics > > > > > > > > 2. The name "DLQ" which in a way it's implying a "queue"... > > > > > > > > which > > > > > can > > > > > > > > be implemented on topic, using a subscription.. > > > > > > > > > > > > > > > > > > > > > > > > -- > > > Zike Yang > > > > -- > Zike Yang
