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

Add balancing test with discovery and connections #387

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ArtDu
Copy link
Contributor

@ArtDu ArtDu commented Apr 26, 2023

I haven't forgotten about:

  • Tests
  • Changelog
  • Documentation
  • Commit messages comply with the guideline
  • Cleanup the code for review. See checklist

Related issues:

@ArtDu ArtDu force-pushed the more_balancing_tests branch 2 times, most recently from aeeddd9 to 7f9bdf4 Compare April 26, 2023 16:21
@ArtDu ArtDu force-pushed the more_balancing_tests branch from 7f9bdf4 to 87468f9 Compare April 26, 2023 16:36
"cartridge.admin_edit_topology({ replicasets = replicasets }) ").join();

// wait until discovery get topology
Thread.sleep(5_000);
Copy link
Collaborator

Choose a reason for hiding this comment

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

That seems highly unstable on different machines. Please use polling (until the cluster gets the proper state) over sleeping.

assertEquals(Arrays.asList(4, 5), getCallCountersPerConnection(getAllConnectionCalls, router));
}

Object routerCallCounterPerConnection = getCallCountersPerConnection(getAllConnectionCalls, routerClient4);
Copy link
Collaborator

Choose a reason for hiding this comment

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

q: Why is it a raw Object?

assertEquals(Arrays.asList(6, 7), getCallCountersPerConnection(getAllConnectionCalls, router));
}

startCartridge();
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does this call do?


@NotNull
private static Object getCallCountersPerConnection(String getAllConnectionCalls, TarantoolClient router) {
List<?> luaResponse = router.eval(getAllConnectionCalls).join();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why isn't it a call or better callForSingleResult/callForMultiResult?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We are showing the readers/users a bad code that they will copy to their projects

@ArtDu
Copy link
Contributor Author

ArtDu commented May 2, 2023

Also we could add parallel round-robin test

@ArtDu ArtDu force-pushed the more_balancing_tests branch from 9c4d441 to c3a874c Compare May 4, 2023 20:54
@ArtDu
Copy link
Contributor Author

ArtDu commented May 5, 2023

Blocked by #400

@ArtDu ArtDu added the blocked Not ready to be implemented label May 5, 2023
@ArtDu ArtDu added blocked Not ready to be implemented and removed blocked Not ready to be implemented labels Jul 3, 2023
@ArtDu ArtDu force-pushed the master branch 2 times, most recently from 367fe33 to f2c5fc3 Compare June 26, 2024 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Not ready to be implemented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants