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

Remove distinction between temporary and permanent object IDs in load balancing #868

Closed
PhilMiller opened this issue Jun 17, 2020 · 7 comments · Fixed by #1226
Closed

Remove distinction between temporary and permanent object IDs in load balancing #868

PhilMiller opened this issue Jun 17, 2020 · 7 comments · Fixed by #1226
Assignees

Comments

@PhilMiller
Copy link
Member

What Needs to be Done?

Is your feature request related to a problem? Please describe.

Describe potential solution outcome

Describe alternatives you've considered

Additional context

@PhilMiller
Copy link
Member Author

Ideally, the IDs would be bijective with the pair {collection ID, index}

@PhilMiller
Copy link
Member Author

@jstrzebonski I'll get more details filled in on this for you later today.

PhilMiller added a commit that referenced this issue Sep 1, 2020
This means that load models, balancers, and load stats files will now
all deal in terms of permanent IDs
@PhilMiller PhilMiller linked a pull request Sep 1, 2020 that will close this issue
@PhilMiller
Copy link
Member Author

I worked through part of the implementation #1017, and ran into the somewhat expected barrier over the use of the ability to extract current node assignments for each object in objGetNode.

The coupled bits of code that run into trouble with this are BaseLB, GreedyLB, ZoltanLB, LBCommKey, and CollectionManager::recordStats

@PhilMiller
Copy link
Member Author

I think some of those usages could be done away with, or have an alternative data source that would give the relevant current node for the object in question.

@nlslatt
Copy link
Collaborator

nlslatt commented Nov 25, 2020

My branch for removing temporary IDs is 868-remove-temporary-ids. I'll try to look at this a bit more on Monday before I'm out.

@nlslatt
Copy link
Collaborator

nlslatt commented Nov 30, 2020

@lifflander I'm ready to hand this off. What I've pushed on 868-remove-temporary-ids almost builds. I'm not sure I understand test_model_comm_overhead.nompi.cc well enough not to break it. I also haven't attempted to update ZoltanLB because you said you were the best person to tackle that.

I left the unique ID part of the struct the same as the old permanent ID, which had bits encoding the rank. There shouldn't be any logic remaining that depends on those bits, so any alternative means of making the IDs unique across ranks should work fine.

@nlslatt
Copy link
Collaborator

nlslatt commented Nov 30, 2020

@lifflander I made a comment to you above but messed up your name, so I'm not sure if you were notified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants