-
Notifications
You must be signed in to change notification settings - Fork 256
[query] Remove BlockMatrix persist from Backend interface #14690
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
Conversation
92da6f0 to
4576acf
Compare
b0be25e to
6038f9b
Compare
4576acf to
8a42e93
Compare
15f2275 to
71fcb35
Compare
341e6a7 to
1682964
Compare
71fcb35 to
360af7c
Compare
1682964 to
b1c0d04
Compare
360af7c to
d16f9c6
Compare
b1c0d04 to
84ddcf3
Compare
d16f9c6 to
783df28
Compare
84ddcf3 to
a2ff477
Compare
783df28 to
7e661e4
Compare
a2ff477 to
df5c723
Compare
7e661e4 to
952b01d
Compare
df5c723 to
adc0602
Compare
952b01d to
a265389
Compare
adc0602 to
24b20f3
Compare
901fafe to
7c3af40
Compare
c5b8c2d to
574082e
Compare
7c3af40 to
dd052e7
Compare
574082e to
2749402
Compare
dd052e7 to
df4033d
Compare
2749402 to
512cbb9
Compare
df4033d to
bd12bb0
Compare
512cbb9 to
387bde2
Compare
bd12bb0 to
bde8755
Compare
387bde2 to
af242c2
Compare
bde8755 to
1ed1a39
Compare
af242c2 to
13a8e28
Compare
1ed1a39 to
aca48d3
Compare
13a8e28 to
97d43aa
Compare
4b0ea93 to
5ff3aee
Compare
5264833 to
057c2a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very straightforward. Looks great.
057c2a1 to
ee87129
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we discussed, I think it's worth considering whether we should just get rid of the SparkBackend specific BlockMatrix persist behavior. But happy to merge this for now to keep our options open!
ee87129 to
ee31986
Compare

This change is split out from a larger refactoring effort on the various Backend
implementations. The goals of this effort are to provide query-level
configuration to the backend that's currently tied to the lifetime of a backend,
reduce code duplication and reduce state duplication.
In this change, I'm removing blockmatrix persist/unpersist from the
Backendinterface by adding
BlockMatrixCache: mutable.Map[String, BlockMatrix]toExecuteContext. The various reader/writer implementations simply fetch theblock matrix from this cache. For the spark backend, this is backed by a cache
whose lifetime is tied to the spark backend. Since block matrices are not
supported in the local and service backends, the cache is an empty map.
Note that block matrix persist is broken in python (#14689)