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

Mistake in documentaion regarding maximum level of QuadPrefixTree(29 not 50) #21191

Closed
chintan-shah-25 opened this issue Oct 31, 2016 · 3 comments
Assignees
Labels
:Analytics/Geo Indexing, search aggregations of geo points and shapes >docs General docs changes good first issue low hanging fruit

Comments

@chintan-shah-25
Copy link

I was looking at this stackoverflow question regarding geo shape mapping. (ES version 2.3)

Documentation says

The maximum amount of levels for the quad trees in Elasticsearch is 50.

but according to source code quadtree maps to lucene PackedQuadPrefixTree which says max levels is 29 and not 50. The code below fails saying "maxLevels of 36 exceeds limit of 29"

PUT test_index

PUT test_index/_mapping/test_type
{
  "properties": {
    "text": {
      "type": "geo_shape",
      "tree": "quadtree",
      "precision": "1mm"
    }
  }
} 

Also there is no documentation regarding 'legacyquadtree' and if you give very high value of tree_levels like below, it throws OOM rather than saying max allowed level is 50

PUT test_index/_mapping/test_type
{
  "properties": {
    "text": {
      "type": "geo_shape",
      "tree": "legacyquadtree",
      "tree_levels": "54354552"
    }
  }
}
@nik9000
Copy link
Member

nik9000 commented Oct 31, 2016

I've assigned this to @nknize because he knows this code well and can figure out what to do more quickly than I can.

@clintongormley clintongormley added >docs General docs changes :Analytics/Geo Indexing, search aggregations of geo points and shapes labels Nov 4, 2016
@nknize nknize added the good first issue low hanging fruit label Mar 26, 2018
@Sue-Gallagher
Copy link
Contributor

Upon further investigation, I've determined that this low hanging fruit isn't quite ripe. The table on https://www.elastic.co/guide/en/elasticsearch/reference/6.2/geo-shape.html is wrong.

  1. Precision is defined as a number plus a value, but the default column (far right) shows only a value, no number. I suspect that the true default is 50m. Please confirm.
  2. tree-levels description clearly states that the default is the default for the chosen prefix tree, but the default column shows 50m. I suspect that the default should be "various"; please confirm.
    and then...
  3. We say that tree-level is the default of the particular prefix tree chosen, but in the descriptions of the prefix trees, we give the maximum value but not the default. If the default is some number below maximum, please provide.

@nknize
Copy link
Contributor

nknize commented Apr 30, 2018

  1. Precision is defined as a number plus a value, but the default column (far right) shows only a value, no number. I suspect that the true default is 50m. Please confirm.

Yes. The default is 50m

  1. I suspect that the default should be "various"

Confirmed

  1. If the default is some number below maximum, please provide.

GeoHashPrefixTree: Default Level == 9
QuadPrefixTree: Default Level == 21

Sue-Gallagher added a commit to Sue-Gallagher/elasticsearch that referenced this issue Apr 30, 2018
Sue-Gallagher added a commit that referenced this issue May 3, 2018
* [DOCS] Changed quad tree max levels to 29. Clears 21191

* Changed QuadPrefixTree max levels to 29 and added defaults. Closes #21191
dnhatn added a commit that referenced this issue May 4, 2018
* master:
  Set the new lucene version for 6.4.0
  [ML][TEST] Clean up jobs in ModelPlotIT
  Upgrade to 7.4.0-snapshot-1ed95c097b (#30357)
  Watcher: Ensure trigger service pauses execution (#30363)
  [DOCS] Added coming qualifiers in changelog
  [DOCS] Commented out empty sections in the changelog to fix the doc build. (#30372)
  Security: reduce garbage during index resolution (#30180)
  Make RepositoriesMetaData contents unmodifiable (#30361)
  Change quad tree max levels to 29. Closes #21191 (#29663)
  Test: use trial license in qa tests with security
  [ML] Add integration test for model plots (#30359)
  SQL: Fix bug caused by empty composites (#30343)
  [ML] Account for gaps in data counts after job is reopened (#30294)
  InternalEngineTests.testConcurrentOutOfOrderDocsOnReplica should use two documents (#30121)
  Change signature of Get Repositories Response (#30333)
  Tests: Use different watch ids per test in smoke test (#30331)
  [Docs] Add term query with normalizer example
  Adds Eclipse config for xpack licence headers (#30299)
  Watcher: Make start/stop cycle more predictable and synchronous (#30118)
  [test] add debug logging for packaging test
  [DOCS] Removed X-Pack Breaking Changes
  [DOCS] Fixes link to TLS LDAP info
  Update versions for start_trial after backport (#30218)
  Packaging: Set elasticsearch user to have non-existent homedir (#29007)
  [DOCS] Fixes broken links to bootstrap user (#30349)
  Fix NPE when CumulativeSum agg encounters null/empty bucket (#29641)
  Make licensing FIPS-140 compliant (#30251)
  [DOCS] Reorganizes authentication details in Stack Overview (#30280)
  Network: Remove http.enabled setting (#29601)
  Fix merging logic of Suggester Options (#29514)
  [DOCS] Adds LDAP realm configuration details (#30214)
  [DOCS] Adds native realm configuration details (#30215)
  ReplicationTracker.markAllocationIdAsInSync may hang if allocation is cancelled (#30316)
  [DOCS] Enables edit links for X-Pack pages (#30278)
  Packaging: Unmark systemd service file as a config file (#29004)
  SQL: Reduce number of ranges generated for comparisons (#30267)
  Tests: Simplify VersionUtils released version splitting (#30322)
  Cancelling a peer recovery on the source can leak a primary permit (#30318)
  Added changelog entry for deb prerelease version change (#30184)
  Convert server javadoc to html5 (#30279)
  Create default ES_TMPDIR on Windows (#30325)
  [Docs] Clarify `fuzzy_like_this` redirect (#30183)
  Post backport of #29658.
  Fix docs of the `_ignored` meta field.
  Remove MapperService#types(). (#29617)
  Remove useless version checks in REST tests. (#30165)
  Add a new `_ignored` meta field. (#29658)
  Move repository-azure fixture test to QA project (#30253)

# Conflicts:
#	buildSrc/version.properties
#	server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java
jasontedor added a commit to martijnvg/elasticsearch that referenced this issue May 6, 2018
* origin/ccr: (166 commits)
  Introduce soft-deletes retention policy based on global checkpoint (elastic#30335)
  Enable MockHttpTransport in ShardChangsIT
  Remove old sha files from dated Lucene snapshot
  Update InternalEngine tests on ccr side for elastic#30121
  Set the new lucene version for 6.4.0
  [ML][TEST] Clean up jobs in ModelPlotIT
  Upgrade to 7.4.0-snapshot-1ed95c097b (elastic#30357)
  Watcher: Ensure trigger service pauses execution (elastic#30363)
  [DOCS] Added coming qualifiers in changelog
  [DOCS] Commented out empty sections in the changelog to fix the doc build. (elastic#30372)
  Security: reduce garbage during index resolution (elastic#30180)
  Make RepositoriesMetaData contents unmodifiable (elastic#30361)
  Change quad tree max levels to 29. Closes elastic#21191 (elastic#29663)
  Test: use trial license in qa tests with security
  [ML] Add integration test for model plots (elastic#30359)
  SQL: Fix bug caused by empty composites (elastic#30343)
  [ML] Account for gaps in data counts after job is reopened (elastic#30294)
  InternalEngineTests.testConcurrentOutOfOrderDocsOnReplica should use two documents (elastic#30121)
  Change signature of Get Repositories Response (elastic#30333)
  Tests: Use different watch ids per test in smoke test (elastic#30331)
  ...
jasontedor added a commit to martijnvg/elasticsearch that referenced this issue May 6, 2018
* origin/ccr: (127 commits)
  Introduce soft-deletes retention policy based on global checkpoint (elastic#30335)
  Enable MockHttpTransport in ShardChangsIT
  Remove old sha files from dated Lucene snapshot
  Update InternalEngine tests on ccr side for elastic#30121
  Set the new lucene version for 6.4.0
  [ML][TEST] Clean up jobs in ModelPlotIT
  Upgrade to 7.4.0-snapshot-1ed95c097b (elastic#30357)
  Watcher: Ensure trigger service pauses execution (elastic#30363)
  [DOCS] Added coming qualifiers in changelog
  [DOCS] Commented out empty sections in the changelog to fix the doc build. (elastic#30372)
  Security: reduce garbage during index resolution (elastic#30180)
  Make RepositoriesMetaData contents unmodifiable (elastic#30361)
  Change quad tree max levels to 29. Closes elastic#21191 (elastic#29663)
  Test: use trial license in qa tests with security
  [ML] Add integration test for model plots (elastic#30359)
  SQL: Fix bug caused by empty composites (elastic#30343)
  [ML] Account for gaps in data counts after job is reopened (elastic#30294)
  InternalEngineTests.testConcurrentOutOfOrderDocsOnReplica should use two documents (elastic#30121)
  Change signature of Get Repositories Response (elastic#30333)
  Tests: Use different watch ids per test in smoke test (elastic#30331)
  ...
jasontedor added a commit to martijnvg/elasticsearch that referenced this issue May 6, 2018
* origin/ccr: (166 commits)
  Introduce soft-deletes retention policy based on global checkpoint (elastic#30335)
  Enable MockHttpTransport in ShardChangsIT
  Remove old sha files from dated Lucene snapshot
  Update InternalEngine tests on ccr side for elastic#30121
  Set the new lucene version for 6.4.0
  [ML][TEST] Clean up jobs in ModelPlotIT
  Upgrade to 7.4.0-snapshot-1ed95c097b (elastic#30357)
  Watcher: Ensure trigger service pauses execution (elastic#30363)
  [DOCS] Added coming qualifiers in changelog
  [DOCS] Commented out empty sections in the changelog to fix the doc build. (elastic#30372)
  Security: reduce garbage during index resolution (elastic#30180)
  Make RepositoriesMetaData contents unmodifiable (elastic#30361)
  Change quad tree max levels to 29. Closes elastic#21191 (elastic#29663)
  Test: use trial license in qa tests with security
  [ML] Add integration test for model plots (elastic#30359)
  SQL: Fix bug caused by empty composites (elastic#30343)
  [ML] Account for gaps in data counts after job is reopened (elastic#30294)
  InternalEngineTests.testConcurrentOutOfOrderDocsOnReplica should use two documents (elastic#30121)
  Change signature of Get Repositories Response (elastic#30333)
  Tests: Use different watch ids per test in smoke test (elastic#30331)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Geo Indexing, search aggregations of geo points and shapes >docs General docs changes good first issue low hanging fruit
Projects
None yet
Development

No branches or pull requests

5 participants