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

Make ip fields backward-compatible at query time. #18593

Merged
merged 1 commit into from May 31, 2016

Conversation

Projects
None yet
2 participants
@jpountz
Copy link
Contributor

commented May 26, 2016

The fact that ip fields used a different doc values representation in 2.x causes
issues when querying 2.x and 5.0 indices in the same request. This changes 2.x
doc values on ip fields/2.x to be hidden behind binary doc values that use the
same encoding as 5.0. This way the coordinating node will be able to merge shard
responses that have different major versions.

One known issue is that this makes sorting/aggregating slower on ip fields for
indices that have been generated with elasticsearch 2.x.

Relates to #17971

@colings86

View changes

core/src/main/java/org/elasticsearch/index/mapper/ip/LegacyIpIndexFieldData.java Outdated
}

public final void clear() {
// can't do

This comment has been minimized.

Copy link
@colings86

colings86 May 31, 2016

Member

By this comment do we mean "there is nothing to do here" or "this should never be called"? If it's the latter should we throw an UnsupportedOperationException instead? If it's the former could we instead change the comment to // nothing to do?

This comment has been minimized.

Copy link
@jpountz

jpountz May 31, 2016

Author Contributor

It is the former. I will fix the comment.

@@ -214,6 +223,99 @@ private int collect(int doc, long ord, long bucket, int lowBound) throws IOExcep
protected abstract void doCollect(LeafBucketCollector sub, int doc, long bucket) throws IOException;
}

static abstract class SortedBinaryRangeLeafCollector extends LeafBucketCollectorBase {

This comment has been minimized.

Copy link
@colings86

colings86 May 31, 2016

Member

I am wondering, given that this duplicates the range binary search from the other implementation, if we should abstract that away somewhere so any fix needed to this logic only needs to be made in one place?

This comment has been minimized.

Copy link
@jpountz

jpountz May 31, 2016

Author Contributor

I thought about it but could not find a way to do it cleanly without relying on boxed Longs, which would make things slower.

This comment has been minimized.

Copy link
@colings86

colings86 May 31, 2016

Member

ok, fair enough

@colings86

This comment has been minimized.

Copy link
Member

commented May 31, 2016

@jpountz I left a couple of comments

@jpountz

This comment has been minimized.

Copy link
Contributor Author

commented May 31, 2016

@colings86 I fixed the comments.

@colings86

This comment has been minimized.

Copy link
Member

commented May 31, 2016

@jpountz thanks, LGTM

Make ip fields backward-compatible at query time. #18593
The fact that ip fields used a different doc values representation in 2.x causes
issues when querying 2.x and 5.0 indices in the same request. This changes 2.x
doc values on ip fields/2.x to be hidden behind binary doc values that use the
same encoding as 5.0. This way the coordinating node will be able to merge shard
responses that have different major versions.

One known issue is that this makes sorting/aggregating slower on ip fields for
indices that have been generated with elasticsearch 2.x.

@jpountz jpountz force-pushed the jpountz:fix/ip_back_compat branch to adf4712 May 31, 2016

@jpountz jpountz merged commit adf4712 into elastic:master May 31, 2016

1 check passed

CLA Commit author is a member of Elasticsearch
Details

@jpountz jpountz deleted the jpountz:fix/ip_back_compat branch May 31, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.