Skip to content

Fix bugs in ImmutableRTree; Merge bytebuffer-collections module into druid-processing#5275

Merged
nishantmonu51 merged 3 commits intoapache:masterfrom
metamx:immutable-rtee-bug
Jan 22, 2018
Merged

Fix bugs in ImmutableRTree; Merge bytebuffer-collections module into druid-processing#5275
nishantmonu51 merged 3 commits intoapache:masterfrom
metamx:immutable-rtee-bug

Conversation

@leventov
Copy link
Copy Markdown
Member

@leventov leventov commented Jan 19, 2018 •
edited
Loading

  • Fix bugs in ImmutableRTree. They are related to incorrect use of ByteBuffer.capacity() instead of remaining().
  • Optimize ImmmutableRTreeObjectStrategy.writeTo();
  • Merge bytebuffer-collections module into druid-processing (I needed to do this in this PR to add a test to ImmutableRTreeTest, which uses GenericIndexed (from druid-processing) at the same time.) So it is a step towards Merge all "core" modules into one "druid-core" module #4312.

@leventov
Fix bugs in ImmutableRTree; optimize ImmmutableRTreeObjectStrategy.wr…
0a5f76a 
…iteTo(); Merge bytebuffer-collections module into druid-processing
@leventov leventov changed the title Fix bugs in ImmutableRTree Fix bugs in ImmutableRTree; Merge bytebuffer-collections module into druid-processing Jan 19, 2018
@leventov leventov added this to the 0.12.0 milestone Jan 19, 2018
gianm
gianm approved these changes Jan 22, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@gianm gianm left a comment

Choose a reason for hiding this comment

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

This LGTM, although I have one question. Is this code in ImmutableRTree also a bug?

Preconditions.checkArgument(data.get(0) == VERSION, "Mismatching versions");

Or should that be data.get(data.position())?

@gianm
Copy link
Copy Markdown
Contributor

gianm commented Jan 22, 2018

LGTM - I am ok to merge this after CI passes.

nishantmonu51
nishantmonu51 approved these changes Jan 22, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@nishantmonu51 nishantmonu51 left a comment

Choose a reason for hiding this comment

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

👍

@nishantmonu51 nishantmonu51 merged commit f99c27e into apache:master Jan 22, 2018
@leventov leventov deleted the immutable-rtee-bug branch January 22, 2018 19:21
@gianm
Copy link
Copy Markdown
Contributor

gianm commented Jan 22, 2018

@leventov could you please backport this to the 0.12.0 branch?

leventov added a commit to metamx/druid that referenced this pull request Jan 23, 2018
@leventov
Fix bugs in ImmutableRTree; Merge bytebuffer-collections module into …
2832051 
…druid-processing (apache#5275)

* Fix bugs in ImmutableRTree; optimize ImmmutableRTreeObjectStrategy.writeTo(); Merge bytebuffer-collections module into druid-processing

* Remove unused declaration

* Fix another bug
@leventov leventov mentioned this pull request Jan 23, 2018
Merged
gianm pushed a commit that referenced this pull request Jan 23, 2018
@leventov @gianm
Fix bugs in ImmutableRTree; Merge bytebuffer-collections module into …
5a8b106 
…druid-processing (#5275) (#5283)

* Fix bugs in ImmutableRTree; optimize ImmmutableRTreeObjectStrategy.writeTo(); Merge bytebuffer-collections module into druid-processing

* Remove unused declaration

* Fix another bug
@gianm gianm mentioned this pull request Aug 30, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gianm gianm gianm approved these changes

@nishantmonu51 nishantmonu51 nishantmonu51 approved these changes

Assignees

No one assigned

Labels

Bug Improvement

Projects

None yet

Milestone

0.12.0

Development

Successfully merging this pull request may close these issues.

3 participants

@leventov @gianm @nishantmonu51