Skip to content

Conversation

@jpountz
Copy link
Contributor

@jpountz jpountz commented May 31, 2017

Currently global ordinals are documented under fielddata. It moves them to
their own file since they also work with doc values and fielddata is on the way
out.

Closes #23101

Currently global ordinals are documented under `fielddata`. It moves them to
their own file since they also work with doc values and fielddata is on the way
out.

Closes elastic#23101
@jpountz jpountz added the >docs General docs changes label May 31, 2017
@jpountz jpountz requested a review from clintongormley May 31, 2017 13:15
Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

LGTM. I left some comments as though you were writing the docs for the first time without realizing you were just moving them. Silly. If you want to rewrite, more power to you. If not, I'll open a followup later with the rewrites.

Global ordinals is a data-structure on top of doc values, that maintains an
incremental numbering for each unique term in a lexicographic order. Each
term has a unique number and the number of term 'A' is lower than the
number of term 'B'. Global ordinals are only supported on
Copy link
Member

Choose a reason for hiding this comment

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

I think "Global ordinals are only used with keyword and text fields. In keyword terms they are available by default but text fields can only use them when fielddata, with all of its associated baggage, is enabled."

Or something like that.

Copy link
Member

Choose a reason for hiding this comment

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

I see that these are just copied. Maybe I can take this comment as a followup.

ordinals to the real term only for the final reduce phase, which combines
results from different shards.

Global ordinals for a specified field are tied to _all the segments of a
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this can be combined with the paragraph above starting with "doc values (and fielddata)". I feel like both are about how you get the global ordinals the the paragraph between the two is about what they are good for.

// CONSOLE
// TEST[s/^/PUT my_index\n/]

This will shift the cost from search-time to refresh-time, ie. Elasticsearch
Copy link
Member

Choose a reason for hiding this comment

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

s/, ie././

content of the index.

If you ever decide that you do not need to run `terms` aggregations on this
field anymore, then you can disable eager loading of global ordinals:
Copy link
Member

Choose a reason for hiding this comment

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

I think this is missing an "at any time" somewhere. Folks are used to mapping things being fixed at field creation but this isn't.

@nik9000
Copy link
Member

nik9000 commented May 31, 2017

I don't think we need a redirect entry for this because the section was an anchor.

@jpountz
Copy link
Contributor Author

jpountz commented May 31, 2017

I pushed a new commit that should address your comments.

incremental numbering for each unique term in a lexicographic order. Each
term has a unique number and the number of term 'A' is lower than the
number of term 'B'. Global ordinals are only supported with
<<keyword,`keyword`>> and <<text,`text`>> fields. In `keyword` fields, they
Copy link
Contributor

Choose a reason for hiding this comment

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

The ip field also uses global ordinals from 5.x, not related to this PR but it reminds me that we should maybe support eager_global_ordinals for ip field too ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. I guess finding the top ip addresses that hit a website is a common use-case.

@jpountz jpountz merged commit ebf806d into elastic:master Jun 1, 2017
@jpountz jpountz deleted the docs/global-ordinals branch June 1, 2017 14:47
jasontedor added a commit to s12v/elasticsearch that referenced this pull request Jun 2, 2017
* master: (62 commits)
  Handle already closed while filling gaps
  [DOCS] Clarify behaviour of scripted-metric arg with empty parent buckets
  [DOCS] Clarify connections and gateway nodes selection in cross cluster search docs (elastic#24859)
  Java api: Remove unneeded getTookInMillis method (elastic#23923)
  Adds nodes usage API to monitor usages of actions (elastic#24169)
  Add superset size to Significant Term REST response (elastic#24865)
  Disallow multiple parent-join fields per mapping (elastic#25002)
  [Test] Remove unused test resources in core (elastic#25011)
  Scripting: Add optional context parameter to put stored script requests (elastic#25014)
  Extract a common base class for scroll executions (elastic#24979)
  Build: fix version sorting
  Build: Move verifyVersions to new branchConsistency task (elastic#25009)
  Add backwards compatibility indices
  Build: improve verifyVersions error message (elastic#25006)
  Add version 5.4.2 constant
  Docs: More search speed advices. (elastic#24802)
  Add version 5.3.3 constant
  Reorganize docs of global ordinals. (elastic#24982)
  Provide the TransportRequest during validation of a search context (elastic#24985)
  [TEST] fix SearchIT assertion to also accept took set to 0
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants