Re: Should the mock consumer call the consumer rebalance listener on rebalance?
I have also created the pull request for the behaviour change. https://github.com/apache/kafka/pull/13455 Thanks again, Dan On Thu, Mar 23, 2023 at 11:40 PM Dan S wrote: > I have created the jira, the PR will follow within the next few days. > https://issues.apache.org/jira/browse/KAFKA-14841 > Thanks again > > On Thu, Mar 23, 2023 at 7:33 PM Philip Nee wrote: > >> Awesome! >> >> On Thu, Mar 23, 2023 at 12:31 PM Dan S wrote: >> >> > I'll pick it up, thanks! >> > >> > On Thu, Mar 23, 2023, 19:27 Philip Nee wrote: >> > >> > > Hey Dan, >> > > >> > > Your analysis looks right. I do see TODO item there to implement the >> > > rebalance callback. Would you like to create a jira issue and work on >> > that? >> > > >> > > Thanks, >> > > P >> > > >> > > On Thu, Mar 23, 2023 at 12:11 PM Dan S wrote: >> > > >> > > > Hi Philip, >> > > > >> > > > Thanks for the quick reply. Yes, it's the MockConsumer, but in our >> case >> > > > we're calling the variant of subscribe that takes a custom >> > > > ConsumerRebalanceListener(which among other things logs when it's >> > > called), >> > > > and we're then calling rebalance (to simulate a rebalance) and >> removing >> > > all >> > > > partitions from the consumer, polling a few times, and then adding >> them >> > > > back. We're noticing our custom listener is never called, which was >> > > > unexpected, but based on the code analysis in the original email >> seems >> > to >> > > > be the current implementation. The question is whether this is >> indeed >> > > > desired behavior. >> > > > >> > > > On Thu, Mar 23, 2023, 18:17 Philip Nee wrote: >> > > > >> > > > > Hey Dan, >> > > > > >> > > > > Thanks for looking into this. Are you talking about >> MockConsumer? If >> > > you >> > > > > invoke subscribe(Collection topics), it actually >> registers a >> > > Noop >> > > > > callback. Perhaps this is what you are seeing? >> > > > > >> > > > > P >> > > > > >> > > > > On Thu, Mar 23, 2023 at 11:11 AM Dan S >> > wrote: >> > > > > >> > > > > > Hello all, >> > > > > > >> > > > > > It seems to me based on reading the code, that the consumer >> > rebalance >> > > > > > listener that is passed into the mock consumer when subscribing >> to >> > a >> > > > > topic >> > > > > > is not actually called when a rebalance is simulated. My >> > > understanding >> > > > is >> > > > > > that the consumer rebalance listener is called from the consumer >> > > > > > coordinator, which is called by kafka consumer. The mock >> consumer >> > > > doesn't >> > > > > > seem to use the consumer coordinator or use any other mechanism >> to >> > > call >> > > > > the >> > > > > > consumer rebalance listener. Is my understanding correct? Would >> it >> > > make >> > > > > > sense to trigger the consumer rebalance listener when rebalance >> is >> > > > > called? >> > > > > > >> > > > > > I would be willing to try to make the patch if the behavior is >> > indeed >> > > > > > currently incorrect/incomplete. >> > > > > > >> > > > > > Thanks, >> > > > > > >> > > > > > Daniel >> > > > > > >> > > > > >> > > > >> > > >> > >> >
Re: Should the mock consumer call the consumer rebalance listener on rebalance?
I have created the jira, the PR will follow within the next few days. https://issues.apache.org/jira/browse/KAFKA-14841 Thanks again On Thu, Mar 23, 2023 at 7:33 PM Philip Nee wrote: > Awesome! > > On Thu, Mar 23, 2023 at 12:31 PM Dan S wrote: > > > I'll pick it up, thanks! > > > > On Thu, Mar 23, 2023, 19:27 Philip Nee wrote: > > > > > Hey Dan, > > > > > > Your analysis looks right. I do see TODO item there to implement the > > > rebalance callback. Would you like to create a jira issue and work on > > that? > > > > > > Thanks, > > > P > > > > > > On Thu, Mar 23, 2023 at 12:11 PM Dan S wrote: > > > > > > > Hi Philip, > > > > > > > > Thanks for the quick reply. Yes, it's the MockConsumer, but in our > case > > > > we're calling the variant of subscribe that takes a custom > > > > ConsumerRebalanceListener(which among other things logs when it's > > > called), > > > > and we're then calling rebalance (to simulate a rebalance) and > removing > > > all > > > > partitions from the consumer, polling a few times, and then adding > them > > > > back. We're noticing our custom listener is never called, which was > > > > unexpected, but based on the code analysis in the original email > seems > > to > > > > be the current implementation. The question is whether this is indeed > > > > desired behavior. > > > > > > > > On Thu, Mar 23, 2023, 18:17 Philip Nee wrote: > > > > > > > > > Hey Dan, > > > > > > > > > > Thanks for looking into this. Are you talking about MockConsumer? > If > > > you > > > > > invoke subscribe(Collection topics), it actually registers > a > > > Noop > > > > > callback. Perhaps this is what you are seeing? > > > > > > > > > > P > > > > > > > > > > On Thu, Mar 23, 2023 at 11:11 AM Dan S > > wrote: > > > > > > > > > > > Hello all, > > > > > > > > > > > > It seems to me based on reading the code, that the consumer > > rebalance > > > > > > listener that is passed into the mock consumer when subscribing > to > > a > > > > > topic > > > > > > is not actually called when a rebalance is simulated. My > > > understanding > > > > is > > > > > > that the consumer rebalance listener is called from the consumer > > > > > > coordinator, which is called by kafka consumer. The mock consumer > > > > doesn't > > > > > > seem to use the consumer coordinator or use any other mechanism > to > > > call > > > > > the > > > > > > consumer rebalance listener. Is my understanding correct? Would > it > > > make > > > > > > sense to trigger the consumer rebalance listener when rebalance > is > > > > > called? > > > > > > > > > > > > I would be willing to try to make the patch if the behavior is > > indeed > > > > > > currently incorrect/incomplete. > > > > > > > > > > > > Thanks, > > > > > > > > > > > > Daniel > > > > > > > > > > > > > > > > > > > > >
Re: Should the mock consumer call the consumer rebalance listener on rebalance?
I'll pick it up, thanks! On Thu, Mar 23, 2023, 19:27 Philip Nee wrote: > Hey Dan, > > Your analysis looks right. I do see TODO item there to implement the > rebalance callback. Would you like to create a jira issue and work on that? > > Thanks, > P > > On Thu, Mar 23, 2023 at 12:11 PM Dan S wrote: > > > Hi Philip, > > > > Thanks for the quick reply. Yes, it's the MockConsumer, but in our case > > we're calling the variant of subscribe that takes a custom > > ConsumerRebalanceListener(which among other things logs when it's > called), > > and we're then calling rebalance (to simulate a rebalance) and removing > all > > partitions from the consumer, polling a few times, and then adding them > > back. We're noticing our custom listener is never called, which was > > unexpected, but based on the code analysis in the original email seems to > > be the current implementation. The question is whether this is indeed > > desired behavior. > > > > On Thu, Mar 23, 2023, 18:17 Philip Nee wrote: > > > > > Hey Dan, > > > > > > Thanks for looking into this. Are you talking about MockConsumer? If > you > > > invoke subscribe(Collection topics), it actually registers a > Noop > > > callback. Perhaps this is what you are seeing? > > > > > > P > > > > > > On Thu, Mar 23, 2023 at 11:11 AM Dan S wrote: > > > > > > > Hello all, > > > > > > > > It seems to me based on reading the code, that the consumer rebalance > > > > listener that is passed into the mock consumer when subscribing to a > > > topic > > > > is not actually called when a rebalance is simulated. My > understanding > > is > > > > that the consumer rebalance listener is called from the consumer > > > > coordinator, which is called by kafka consumer. The mock consumer > > doesn't > > > > seem to use the consumer coordinator or use any other mechanism to > call > > > the > > > > consumer rebalance listener. Is my understanding correct? Would it > make > > > > sense to trigger the consumer rebalance listener when rebalance is > > > called? > > > > > > > > I would be willing to try to make the patch if the behavior is indeed > > > > currently incorrect/incomplete. > > > > > > > > Thanks, > > > > > > > > Daniel > > > > > > > > > >
Re: Should the mock consumer call the consumer rebalance listener on rebalance?
Hi Philip, Thanks for the quick reply. Yes, it's the MockConsumer, but in our case we're calling the variant of subscribe that takes a custom ConsumerRebalanceListener(which among other things logs when it's called), and we're then calling rebalance (to simulate a rebalance) and removing all partitions from the consumer, polling a few times, and then adding them back. We're noticing our custom listener is never called, which was unexpected, but based on the code analysis in the original email seems to be the current implementation. The question is whether this is indeed desired behavior. On Thu, Mar 23, 2023, 18:17 Philip Nee wrote: > Hey Dan, > > Thanks for looking into this. Are you talking about MockConsumer? If you > invoke subscribe(Collection topics), it actually registers a Noop > callback. Perhaps this is what you are seeing? > > P > > On Thu, Mar 23, 2023 at 11:11 AM Dan S wrote: > > > Hello all, > > > > It seems to me based on reading the code, that the consumer rebalance > > listener that is passed into the mock consumer when subscribing to a > topic > > is not actually called when a rebalance is simulated. My understanding is > > that the consumer rebalance listener is called from the consumer > > coordinator, which is called by kafka consumer. The mock consumer doesn't > > seem to use the consumer coordinator or use any other mechanism to call > the > > consumer rebalance listener. Is my understanding correct? Would it make > > sense to trigger the consumer rebalance listener when rebalance is > called? > > > > I would be willing to try to make the patch if the behavior is indeed > > currently incorrect/incomplete. > > > > Thanks, > > > > Daniel > > >
Should the mock consumer call the consumer rebalance listener on rebalance?
Hello all, It seems to me based on reading the code, that the consumer rebalance listener that is passed into the mock consumer when subscribing to a topic is not actually called when a rebalance is simulated. My understanding is that the consumer rebalance listener is called from the consumer coordinator, which is called by kafka consumer. The mock consumer doesn't seem to use the consumer coordinator or use any other mechanism to call the consumer rebalance listener. Is my understanding correct? Would it make sense to trigger the consumer rebalance listener when rebalance is called? I would be willing to try to make the patch if the behavior is indeed currently incorrect/incomplete. Thanks, Daniel
Re: Ci stability
Thanks Colin, I have a draft PR open which I occasionally check on and disable the failing tests, I'll update it and see if it passes. Thanks, Daniel Scanteianu On Mon, Dec 5, 2022, 18:02 Colin McCabe wrote: > FYI, there was a memory leak that affected some of the tests which was > fixed recently, so hopefully stability will improve a bit. See KAFKA-14433 > for details. > > best, > Colin > > On Thu, Nov 24, 2022, at 12:48, John Roesler wrote: > > Hi Dan, > > > > I’m not sure if there’s a consistently used tag, but I’ve gotten good > > mileage out of just searching for “flaky” or “flaky test” in Jira. > > > > If you’re thinking about filing a ticket for a specific test failure > > you’ve seen, I’ve also usually been able to find out whether there’s > > already a ticket by searching for the test class or method name. > > > > People seem to typically file tickets with “flaky” in the title and > > then the test name. > > > > Thanks again for your interest in improving the situation! > > -John > > > > On Thu, Nov 24, 2022, at 10:08, Dan S wrote: > >> Thanks for the reply John! Is there a jira tag or view or something that > >> can be used to find all the failing tests and maybe even try to fix them > >> (even if fix just means extending a timeout)? > >> > >> > >> > >> On Thu, Nov 24, 2022, 16:03 John Roesler wrote: > >> > >>> Hi Dan, > >>> > >>> Thanks for pointing this out. Flaky tests are a perennial problem. We > >>> knock them out every now and then, but eventually more spring up. > >>> > >>> I’ve had some luck in the past filing Jira tickets for the failing > tests > >>> as they pop up in my PRs. Another thing that seems to motivate people > is to > >>> open a PR to disable the test in question, as you mention. That can be > a > >>> bit aggressive, though, so it wouldn’t be my first suggestion. > >>> > >>> I appreciate you bringing this up. I agree that flaky tests pose a > risk to > >>> the project because it makes it harder to know whether a PR breaks > things > >>> or not. > >>> > >>> Thanks, > >>> John > >>> > >>> On Thu, Nov 24, 2022, at 02:38, Dan S wrote: > >>> > Hello all, > >>> > > >>> > I've had a pr that has been open for a little over a month (several > >>> > feedback cycles happened), and I've never seen a fully passing build > >>> (tests > >>> > in completely different parts of the codebase seemed to fail, often > >>> > timeouts). A cursory look at open PRs seems to indicate that mine is > not > >>> > the only one. I was wondering if there is a place where all the flaky > >>> tests > >>> > are being tracked, and if it makes sense to fix (or at least > temporarily > >>> > disable) them so that confidence in new PRs could be increased. > >>> > > >>> > Thanks, > >>> > > >>> > Dan > >>> >
Re: Ci stability
Thanks for the reply John! Is there a jira tag or view or something that can be used to find all the failing tests and maybe even try to fix them (even if fix just means extending a timeout)? On Thu, Nov 24, 2022, 16:03 John Roesler wrote: > Hi Dan, > > Thanks for pointing this out. Flaky tests are a perennial problem. We > knock them out every now and then, but eventually more spring up. > > I’ve had some luck in the past filing Jira tickets for the failing tests > as they pop up in my PRs. Another thing that seems to motivate people is to > open a PR to disable the test in question, as you mention. That can be a > bit aggressive, though, so it wouldn’t be my first suggestion. > > I appreciate you bringing this up. I agree that flaky tests pose a risk to > the project because it makes it harder to know whether a PR breaks things > or not. > > Thanks, > John > > On Thu, Nov 24, 2022, at 02:38, Dan S wrote: > > Hello all, > > > > I've had a pr that has been open for a little over a month (several > > feedback cycles happened), and I've never seen a fully passing build > (tests > > in completely different parts of the codebase seemed to fail, often > > timeouts). A cursory look at open PRs seems to indicate that mine is not > > the only one. I was wondering if there is a place where all the flaky > tests > > are being tracked, and if it makes sense to fix (or at least temporarily > > disable) them so that confidence in new PRs could be increased. > > > > Thanks, > > > > Dan >
Ci stability
Hello all, I've had a pr that has been open for a little over a month (several feedback cycles happened), and I've never seen a fully passing build (tests in completely different parts of the codebase seemed to fail, often timeouts). A cursory look at open PRs seems to indicate that mine is not the only one. I was wondering if there is a place where all the flaky tests are being tracked, and if it makes sense to fix (or at least temporarily disable) them so that confidence in new PRs could be increased. Thanks, Dan
Re: [DISCUSS] KIP-886 Add Client Producer and Consumer Builders
Hello all, I've gotten great feedback from Knowles here, and from Luke Chen on the jira, so thanks to both so much. This is my first KIP, and I'm pretty new to contributing to kafka, so I'd like to learn a little bit more about the process and the way things usually work. Should I open a PR first? Should I simply wait for discussion comments or votes (I've gotten no votes yet)? Thanks so much, Dan On Fri, Nov 11, 2022 at 1:30 AM Knowles Atchison Jr wrote: > This would be helpful. For our own client library wrappers we implemented > this functionality for any type with defaults for and > consumers/producers. > > On Thu, Nov 10, 2022, 6:35 PM Dan S wrote: > > > Hello all, > > > > I think that adding builders for the producer and the consumer in kafka > > client would make it much easier for developers to instantiate new > > producers and consumers, especially if they are using an IDE with > > intellisense, and using the IDE to navigate to the documentation which > > could be added to the builder's withXYZ methods. > > > > Please let me know if you have any comments, questions, or suggestions! > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-886%3A+Add+Client+Producer+and+Consumer+Builders > > > > Thanks, > > > > Dan > > >
[VOTE] KIP-886 Add Client Producer and Consumer Builders
Hello all, I think that adding builders for the producer and the consumer in kafka client would make it much easier for developers to instantiate new producers and consumers, especially if they are using an IDE with intellisense, and using the IDE to navigate to the documentation which could be added to the builder's withXYZ methods. Please let me know if you have any comments, questions, or suggestions in the discussion thread, or vote here! https://cwiki.apache.org/confluence/display/KAFKA/KIP-886 %3A+Add+Client+Producer+and+Consumer+Builders Thanks, Dan
[DISCUSS] KIP-886 Add Client Producer and Consumer Builders
Hello all, I think that adding builders for the producer and the consumer in kafka client would make it much easier for developers to instantiate new producers and consumers, especially if they are using an IDE with intellisense, and using the IDE to navigate to the documentation which could be added to the builder's withXYZ methods. Please let me know if you have any comments, questions, or suggestions! https://cwiki.apache.org/confluence/display/KAFKA/KIP-886%3A+Add+Client+Producer+and+Consumer+Builders Thanks, Dan
Review request - PR#12753
Hello, I would really appreciate another review on https://github.com/apache/kafka/pull/12753/files as I think it would be great to add a bit more documentation on the behaviour of seek, as well as some tests around invalid offsets (I found this very confusing when developing for it). Thanks, Dan
request to contribute to kafka
Hello, I would like to contribute to kafka, my wiki id, jira id, and github username are all "scanteianu" Thanks, Dan
review request: #12753 (small pr)
Hello all, I'd love to get this reviewed and ideally merged, it's just adding some documentation around seek() that I would have found helpful while developing. The tests which are failing seem to be from a completely different part of the codebase (mostly around consensus mechanism). https://github.com/apache/kafka/pull/12753 Thank you, Dan
Odd behaviour (bug?) - seek with "latest" offset reset strategy
Hello, I opened a PR to add slightly more detailed documentation to seek(), as I had spent hours googling when I wanted to use it. After some great reviews from @showuon, I added some integration tests, and I noticed very odd behaviour: https://github.com/apache/kafka/pull/12753/files#r1007760016 In these tests, we start with 10 messages going to a topic partition, and getting consumed. Then, we seek to an invalid offset. Based on https://medium.com/lydtech-consulting/kafka-consumer-auto-offset-reset-d3962bad2665 The offset reset should kick in, and as new records show up, they should be read. What I have observed (the tests seem to pass locally is): After the above, if I poll, I get an empty list, which makes sense (we're at the end, waiting for new messages) If I seek to offset 17, and then add 10 more messages, the next message I get is offset 17, which is not what the link says, but sort of makes sense, because it's what I asked for. If, however, I seek to 27, and add 10 messages, the next offset I get is 0, which seeks plain wrong. We're neither at the old end (offset 10), or at the new end (offset 20), or where we asked to be (offset 27). Am I doing something wrong/missing something, or is there a bug, and if so, what should I do (file a jira, add a fix to the pr, open a new pr?). What is the desired behaviour? Is it to get message at offset 11 in both cases? Thanks, Dan
Odd behaviour (bug?) - seek with "latest" offset reset strategy
Hello, I opened a PR to add slightly more detailed documentation to seek(), as I had spent hours googling when I wanted to use it. After some great reviews from @showuon, I added some integration tests, and I noticed very odd behaviour: https://github.com/apache/kafka/pull/12753/files#r1007760016 In these tests, we start with 10 messages going to a topic partition, and getting consumed. Then, we seek to an invalid offset. Based on https://medium.com/lydtech-consulting/kafka-consumer-auto-offset-reset-d3962bad2665 The offset reset should kick in, and as new records show up, they should be read. What I have observed (the tests seem to pass locally is): After the above, if I poll, I get an empty list, which makes sense (we're at the end, waiting for new messages) If I seek to offset 17, and then add 10 more messages, the next message I get is offset 17, which is not what the link says, but sort of makes sense, because it's what I asked for. If, however, I seek to 27, and add 10 messages, the next offset I get is 0, which seeks plain wrong. We're neither at the old end (offset 10), or at the new end (offset 20), or where we asked to be (offset 27). Am I doing something wrong/missing something, or is there a bug, and if so, what should I do (file a jira, add a fix to the pr, open a new pr?). What is the desired behaviour? Is it to get message at offset 11 in both cases? Thanks, Dan
