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

scheduler: overhaul util.go/tasksUpdated #16290

Closed
shoenig opened this issue Mar 1, 2023 · 0 comments · Fixed by #16421
Closed

scheduler: overhaul util.go/tasksUpdated #16290

shoenig opened this issue Mar 1, 2023 · 0 comments · Fixed by #16421
Assignees
Labels
stage/accepted Confirmed, and intend to work on. No timeline committment though. theme/cleanup type/bug

Comments

@shoenig
Copy link
Member

shoenig commented Mar 1, 2023

tasksUpdated is a critical block of code that determines whether a replacement alloc will be necessary upon diffing the task group of two jobs. However it provides no trace of why a task group is determined to need replacing - it merely returns a boolean and logs nothing.

Also we should stop using reflect.DeepEqual for comparison here (or anywhere). It does not do the "right" thing when comparing e.g. nil vs empty slices, and has other gotchas. We should write Equal methods for every type, or defer to go-cmp with custom comparators.

Context: in investigating #16238 it seems something has broken in-place updates for services but it's not clear what.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stage/accepted Confirmed, and intend to work on. No timeline committment though. theme/cleanup type/bug
Projects
Development

Successfully merging a pull request may close this issue.

2 participants