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

Should have native .NET output binding support #11

Closed
1 of 3 tasks
ryancrawcour opened this issue Mar 18, 2019 · 18 comments
Closed
1 of 3 tasks

Should have native .NET output binding support #11

ryancrawcour opened this issue Mar 18, 2019 · 18 comments
Milestone

Comments

@ryancrawcour
Copy link
Contributor

ryancrawcour commented Mar 18, 2019

Must be able to write output to a Kafka topic using output bindings

  • Output binding
  • Expose important configuration options for producers
  • Add unit tests
@ryancrawcour ryancrawcour added this to the P0 milestone Mar 18, 2019
@ryancrawcour ryancrawcour changed the title Must have native .NET binding support Should have native .NET output binding support Mar 25, 2019
@ryancrawcour ryancrawcour modified the milestones: P0, P1 Mar 25, 2019
This was referenced Mar 30, 2019
@brandonh-msft brandonh-msft added the question Further information is requested label Apr 2, 2019
@brandonh-msft
Copy link
Member

To help in somebody picking this up, can we define "important" in this issue?

@fbeltrao
Copy link
Contributor

fbeltrao commented Apr 2, 2019

My suggestions would be for you to take a look at the librdkafka and Confluent.Kafka configuration docs and propose what settings should be exposed in the function.

I would start with settings that affect performance and throughput. Settings about max message size could be important too, in case we need to support topics with large messages.

At the end we should add the newly exposed options in our documentation section.

Does it help @brandonh-msft ?

@brandonh-msft
Copy link
Member

perfect, thanks

@brandonh-msft brandonh-msft removed the question Further information is requested label Apr 2, 2019
@ryancrawcour
Copy link
Contributor Author

@brandonh-msft is this* something you're going to tackle?

  • take a look at the librdkafka and Confluent.Kafka configuration docs and propose what settings should be exposed in the function.
    At the end we should add the newly exposed options in our documentation section.

@brandonh-msft
Copy link
Member

you can put this on my backlog but i'm tackling #44 first

@ryancrawcour
Copy link
Contributor Author

Ok, assigned to you

@brandonh-msft
Copy link
Member

brandonh-msft commented Apr 3, 2019

K so right now, looking at Global configuration properties in librdkafka docs I'm thinking:

property proposed name description
message.max.bytes MaxMessageBytes Maximum transmit message size.
enable.auto.commit AutoCommit Automatically and periodically commit offsets in the background.
auto.commit.interval.ms AutoCommitIntervalMs The frequency in milliseconds that the consumer offsets are commited (written) to offset storage. (0 = disable)
batch.num.messages BatchSize Maximum number of messages batched in one MessageSet.

It's not clear to me if some of those are implementations we'd have to code up (autocommit, for instance) or if it's something we specify on the msg that's sent and the protocol/receiver takes care of it. Would be good to get some clarity there if you guys know off the top of your head.

Nothing else is standing out as either applicable to sending messages, or particularly useful.

@fbeltrao
Copy link
Contributor

fbeltrao commented Apr 3, 2019

Are those consumer or producer related settings? It is unclear to me.
Regarding commit configuration: we are doing manual commits, therefore enable.auto.commit and auto.commit.interval.ms will not be used/exposed.

@brandonh-msft
Copy link
Member

yeah TBH i'm not sure either; i agree it's not super clear in the docs whether it's producer related or not. I just kinda guessed.

So now considering just:

property proposed name description
message.max.bytes MaxMessageBytes Maximum transmit message size.
batch.num.messages BatchSize Maximum number of messages batched in one MessageSet.

@ryancrawcour
Copy link
Contributor Author

Ok. let's start with these. and when people request others or want extra features that are unblocked by exposing others, then we can go add them.

@fbeltrao
Copy link
Contributor

fbeltrao commented Apr 3, 2019

Not sure if we should expose:
enable.idempotence

source: https://github.com/edenhill/librdkafka/blob/master/CONFIGURATION.md

@ryancrawcour
Copy link
Contributor Author

Can someone please update the docs and send a PR for bullet point 2 then. sounds like it's done.

@brandonh-msft
Copy link
Member

brandonh-msft commented Apr 4, 2019

After some more digging found these are all the knobs even possible to set w/ the Kafka .Net SDK - do we just expose them all?
image

@fbeltrao
Copy link
Contributor

fbeltrao commented Apr 4, 2019

We already have so many, adding just for the sake of having all might in turn cause the configuration to be over complicated.

@brandonh-msft
Copy link
Member

fair enough - it doesn't look like, though, i can tweak the pieces I called out earlier after looking at what the .Net SDK exposes in ProducerConfig - which of the above do you think we'd want to expose, then? Perhaps just the EnableIdempotence? I can see value in the Timeouts, BatchNumMessages, and MessageSendMaxRetries, though.

@fbeltrao
Copy link
Contributor

fbeltrao commented Apr 4, 2019

I like your proposal. For Idempotence we need to check how we handle errors in our producer

@ryancrawcour
Copy link
Contributor Author

Yeah, go for the simplest config that enables the majority of scenarios. We can start exposing more config options as developers using this start requesting additional features. Too many knobs and dials get confusing

@ryancrawcour
Copy link
Contributor Author

@brandonh-msft What's the status of this? In progress? Or complete?

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

No branches or pull requests

3 participants