You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "AyoubOm (via GitHub)" <gi...@apache.org> on 2024/02/20 17:41:47 UTC
[PR] KAFKA-14747: record discarded FK join subscription responses [kafka]
AyoubOm opened a new pull request, #15395:
URL: https://github.com/apache/kafka/pull/15395
*As described in KAFKA-14747, we are not recording discarded FK join responses in case of receiving a join response for an old record (whose hash value has changed in the left table). This PR adds the record to dropped records sensor*
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-14747: record discarded FK join subscription responses [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax merged PR #15395:
URL: https://github.com/apache/kafka/pull/15395
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-14747: record discarded FK join subscription responses [kafka]
Posted by "AyoubOm (via GitHub)" <gi...@apache.org>.
AyoubOm commented on PR #15395:
URL: https://github.com/apache/kafka/pull/15395#issuecomment-1978135956
> > Another point, I think there are some missing tests in the foreignkeyjoin package, and problems in naming a test class. For example, ForeignTableJoinProcessorSupplierTest is not testing ForeignTableJoinProcessorSupplier but SubscriptionJoinProcessorSupplier. Hence, I think the ForeignTableJoinProcessorSupplier is not tested.
>
> Thanks for pointing this out. Would you like to do a follow up PR and rename the test? And maybe also close this test gap (of not, we can also just file a ticket for this for now.)
Sure, I will look into that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-14747: record discarded FK join subscription responses [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on PR #15395:
URL: https://github.com/apache/kafka/pull/15395#issuecomment-1977692891
Also, thanks for the PR! Merged to `trunk`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-14747: record discarded FK join subscription responses [kafka]
Posted by "AyoubOm (via GitHub)" <gi...@apache.org>.
AyoubOm commented on PR #15395:
URL: https://github.com/apache/kafka/pull/15395#issuecomment-1963731421
@mjsax Please have a look when you have some time
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-14747: record discarded FK join subscription responses [kafka]
Posted by "AyoubOm (via GitHub)" <gi...@apache.org>.
AyoubOm commented on PR #15395:
URL: https://github.com/apache/kafka/pull/15395#issuecomment-1976087470
> Thanks for the PR! Overall LGTM, but we should add a unit test for the newly added sensor.
Thanks @mjsax for your review. I added tests of the dropped record sensor (I believed we didn't have access to metrics from the context but we have indeed).
Another point, I think there are some missing tests in the foreignkeyjoin package, and problems in naming a test class. For example, `ForeignTableJoinProcessorSupplierTest` is not testing `ForeignTableJoinProcessorSupplier` but `SubscriptionJoinProcessorSupplier`. Hence, I think the ForeignTableJoinProcessorSupplier is not tested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-14747: record discarded FK join subscription responses [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on PR #15395:
URL: https://github.com/apache/kafka/pull/15395#issuecomment-1977688896
> Another point, I think there are some missing tests in the foreignkeyjoin package, and problems in naming a test class. For example, ForeignTableJoinProcessorSupplierTest is not testing ForeignTableJoinProcessorSupplier but SubscriptionJoinProcessorSupplier. Hence, I think the ForeignTableJoinProcessorSupplier is not tested.
Thanks for pointing this out. Would you like to do a follow up PR and rename the test? And maybe also close this test gap (of not, we can also just file a ticket for this for now.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org