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

[server] Increase retry limit in metadata fetch #1241

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

majisourav99
Copy link
Contributor

@majisourav99 majisourav99 commented Oct 16, 2024

##[server] Adjust retry limit in metadata fetch

Increase retry limit in for non-blocking metadata fetch method inside getTopicPartitionEndOffSet from 5 retries to 10 as too few retries made some tests fails.

How was this PR tested?

Does this PR introduce any user-facing changes?

  • No. You can skip the rest of this section.
  • Yes. Make sure to explain your proposed changes and call out the behavior change.

Copy link
Contributor

@sushantmane sushantmane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please commit message to better reflect which metadata limit and where?

@sushantmane
Copy link
Contributor

Let’s update the commit title as well. Could you also add more details to the PR, such as the number of iterations you ran to confirm the fix?

@FelixGV
Copy link
Contributor

FelixGV commented Oct 16, 2024

Is this supposed to help with com.linkedin.venice.endToEnd.DaVinciClientMemoryLimitTest > testDaVinciMemoryLimitShouldFailLargeDataPush? I see that it still failed in 2/4 permutations in the first attempt, then succeeded on the retry... maybe it fails less often though? IDK...

@FelixGV
Copy link
Contributor

FelixGV commented Oct 18, 2024

Probably should revert the whole file to what it was before my commit, not just the DataProvider part. Unless you intend to test the contribution of each change individually? I think bumping the RF from 1 to 2 may have been significant. The restructuring of setup/close code into Before/AfterMethod, IDK how much it actually helped, maybe it did, maybe not.

@majisourav99
Copy link
Contributor Author

Probably should revert the whole file to what it was before my commit, not just the DataProvider part. Unless you intend to test the contribution of each change individually? I think bumping the RF from 1 to 2 may have been significant. The restructuring of setup/close code into Before/AfterMethod, IDK how much it actually helped, maybe it did, maybe not.

Yes, was going to change the RF 1 next. setup/close I am not sure worth a try.

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

Successfully merging this pull request may close these issues.

3 participants