MINOR: Fix regression in MM2 task forwarding introduced by KAFKA-14783#13548
MINOR: Fix regression in MM2 task forwarding introduced by KAFKA-14783#13548C0urante merged 1 commit intoapache:trunkfrom
Conversation
The DistributedHerder was computing the forwarded URL for publishing task configs incorrectly leading to 404s in MM2 distributed mode. Signed-off-by: Greg Harris <greg.harris@aiven.io>
|
The PR build has 95 failures, many of them in a related area (at least on the surface):
How come we merged this without mentioning why it's ok to do so? |
|
@ijuma The scope of this PR was small, addressing just one issue. We certainly have several flakey and failing tests that need to be addressed during this test stabilization period. Here are the relevant tickets i've found:
And outside of the connect/mirrormaker area:
|
|
But how do we even know that this is helping vs making it worse with so many failures? |
|
@ijuma I proposed this PR in the form I did because it restored a piece of code that was unintentionally changed. From an end user-perspective, the unintentional change was certainly bad, and reverting that change (in this PR) is certainly good. I'm sorry if the PR description did not make that clear, and I'll try to improve in the future. I have looked into the other changes and identified their causes to be something distinct from this regression:
All in all, I saw an obvious typo in a previous PR, and opened a follow-up to fix it. I did not want to issue a "Fix every Connect test failure" as I think that would be harder to review and less modular if reverts needed to be made in the future. |
|
@gharris1727 I think what you did was fine. :) My concern is more on how we decided that it was ok to merge. I looked a bit more and I think the issues potentially started around here for a bunch of the tests: And it looks like a similar pattern happened there where we merged without a detailed analysis of the failures. |
3 participants
The DistributedHerder was computing the forwarded URL for publishing task configs incorrectly leading to 404s in MM2 distributed mode.
This regression appears in #13424 and presently only exists on trunk. This manifests as a return to pre-KIP-710 behavior, and causes the DedicatedMirrorIntegrationTest to fail whenever forwarding happens, making the test flake in >50% of runs.
It appears to be just a typo and not an intended change, and was hidden by the github diff when this function was split into two parts.
Committer Checklist (excluded from commit message)