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

Is affinity() supposed to return a Pair{OSProc, UInt64} or a Vector{Pair{OSProc, UInt64}}? #295

Closed
mattwigway opened this issue Oct 20, 2021 · 3 comments

Comments

@mattwigway
Copy link
Contributor

The definition of affinity for a DRef returns a pair, while the definition for a FileRef returns a singleton Vector{Pair}. This is causing a boundserror in Sch.jl which calls affinity(...)[2] and crashes when affinity returns a singleton vector. This is causing the tests for JuliaDB to fail on Julia 1.7 (though they work on earlier versions, perhaps those versions are using the DRef codepath). Based on the code it seems like affinity should return a Pair rather than a Vector{Pair}, but wanted to confirm before making a PR.

@mattwigway
Copy link
Contributor Author

mattwigway commented Oct 20, 2021

@jpsamaroo
Copy link
Member

Yeah, I chose to change the definition of affinity() for #221 because I felt that returning affinities for every worker who might have the DRef cached would be excessive (even though the old definition didn't actually do that, but it should have to fulfill the contract of affinity()). FileRef should probably be updated to just return a single pair for the owner. Note that #289 will probably end up using its own variant of FileRef, and may obviate the need for JuliaDB to know anything about where data resides (in-memory, on disk, etc.).

I'd be happy to accept a PR adjusting affinity(::FileRef) to just return a single pair, OSProc(1) => size, since we don't currently use the first item of the pair anyway.

@mattwigway
Copy link
Contributor Author

Great, will prepare a PR.

jpsamaroo added a commit that referenced this issue Oct 22, 2021
affinity now returns Pair not Vector{Pair}, fixes #295
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants