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

Do not pass back RTrees to clients unless it's needed #5265

Merged
merged 10 commits into from
Sep 4, 2024

Conversation

robertbindar
Copy link
Contributor

@robertbindar robertbindar commented Aug 28, 2024

More details here: https://app.shortcut.com/tiledb-inc/story/54168/figure-out-correct-serialization-of-rtrees-in-rest
RTrees are only needed in global order writes, unordered partial attribute writes and FragmentInfo APIs.

I validated this change against a local REST server and arrays can be read with no issues by older clients.


TYPE: IMPROVEMENT
DESC: Do not pass back RTrees to clients.

@robertbindar robertbindar force-pushed the rbin/ch53761/move_loadedmeta_query branch 2 times, most recently from 207bde0 to 4c42aa2 Compare August 29, 2024 15:14
@robertbindar robertbindar force-pushed the rbin/ch53761/move_loadedmeta_query branch from 147525a to 3b25115 Compare September 2, 2024 14:20
@robertbindar robertbindar marked this pull request as ready for review September 2, 2024 14:20
@robertbindar robertbindar changed the title [NOT_FOR_MERGE] test loaded_rtree checks Do not pass back RTrees to clients, except GOW or FragmentInfo Sep 2, 2024
@robertbindar robertbindar changed the title Do not pass back RTrees to clients, except GOW or FragmentInfo Do not pass back RTrees to clients unless it's needed Sep 2, 2024
@KiterLuc
Copy link
Contributor

KiterLuc commented Sep 2, 2024

When you say: "I validated this change against a local REST server and arrays can be read with no issues by older clients.", you mean with older python clients?

@robertbindar
Copy link
Contributor Author

When you say: "I validated this change against a local REST server and arrays can be read with no issues by older clients.", you mean with older python clients?

Older python client with and without array open and query v3 enabled.

Copy link
Member

@ypatia ypatia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work, thanks for fixing this long standing pain!

@KiterLuc KiterLuc merged commit a14f9e1 into dev Sep 4, 2024
63 checks passed
@KiterLuc KiterLuc deleted the rbin/ch53761/move_loadedmeta_query branch September 4, 2024 13:18
ypatia added a commit that referenced this pull request Nov 14, 2024
ypatia added a commit that referenced this pull request Nov 15, 2024
This reverts #5265 although it
has introduced a good change: only serialize rtrees when absolutely
needed. However, the problem is that the server will be in 2.26 and will
expect to serialize rtrees that have not been deserialized before and
will fail. This will not allow the release to get verified completely.

---
TYPE: IMPROVEMENT
DESC: Revert selective Rtree serialization to allow release testing
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