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

v2.1: Add not unique leader discovery (backport of #3546) #3658

Merged
merged 1 commit into from
Nov 22, 2024

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Nov 15, 2024

Problem

Currently, TpuClient next API allows to get only unique future leaders. For example, if the leader schedule is [L1, L1, L1, L1, L2, L2, L2, L2, L1, L1, L1, L1], it will return [L1, L2]. While for the internal logic of tpu-client-next it is desirable to get [L1, L2, L1] instead.

Summary of Changes


This is an automatic backport of pull request #3546 done by [Mergify](https://mergify.com).

* rename get_leader_sockets to get_unique_leader_sockets

* Add methods for getting not unique tpu sockets

(cherry picked from commit dffcdb4)
@mergify mergify bot requested a review from a team as a code owner November 15, 2024 07:33
@alessandrod
Copy link

The naming of these methods is all wrong, but hear me out: the PR renames an existing method - get_leader_sockets() - to get_unique_leader_sockets() and updates all the callers. No logic changes Then introduces get_leader_sockets(), but with a slightly more useful implementation. The new method is only used by the -next crate which isn't used by anything (yet).

@KirillLykov it'd be great if in a follow up PR you could add example results of the two methods, and rename _sockets to _socket_addrs or something - I thought we were creating sockets and almost had an aneurysm.

@alessandrod alessandrod merged commit cefd3f1 into v2.1 Nov 22, 2024
28 checks passed
@alessandrod alessandrod deleted the mergify/bp/v2.1/pr-3546 branch November 22, 2024 12:01
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