optimize source filtering in SourceFieldMapper by mushao999 · Pull Request #81970 · elastic/elasticsearch

mushao999 · 2021-12-21T07:07:09Z

@nik9000 and @weizijun did a great job in introducing a memory efficient xcontent filtering approach and benchmarked its peformance (#77154 #81575). From their contribution, source filtering will be faster significantly.
This PR apply this approach into SourceFilterMapper where map filtering is used to include or exclude fields.
We did a benchmark using esrally, and the details list as follow:

Cluster: 3 nodes 12Core 24G(Heap size)
Rally version: 2.3.0, param:--track=nyc_taxis --challenge=append-no-conflicts-index-only --client-options="timeout:60,basic_auth_user:'elastic',basic_auth_password:'Admin@gal'" --track-params='{"bulk_size": 1000, "index_settings": {"index.number_of_replicas": "1", "index.number_of_shards": "6"}, "bulk_indexing_clients": 10}'
Modified nyc-taxis's index.json, add "exludes": ["surcharge", "pickup_location"]
Result:
- ParseFilter indexing Mean Throughput 59029.2 docs/s, 10% more than that of MapFilter which is 54825.9docs/s
- ParseFilter indexing 50th percentile latency 144.171ms almost the same with that of MapFilter which is 145.753ms
- ParseFilter indexing 99th percentile latency 1106.4ms, 25% faster then that of MapFilter which is 1464.82ms

So we believe indexing operation with source filtering will be more memory efficient after this PR being applied.

…ng config

mushao999 · 2021-12-21T07:17:25Z

Rally Result of MapFilter

------------------------------------------------------
    _______             __   _____
   / ____(_)___  ____ _/ /  / ___/_________  ________
  / /_  / / __ \/ __ `/ /   \__ \/ ___/ __ \/ ___/ _ \
 / __/ / / / / / /_/ / /   ___/ / /__/ /_/ / /  /  __/
/_/   /_/_/ /_/\__,_/_/   /____/\___/\____/_/   \___/
------------------------------------------------------

|                                                         Metric |                 Task |       Value |   Unit |
|---------------------------------------------------------------:|---------------------:|------------:|-------:|
|                     Cumulative indexing time of primary shards |                      |     202.422 |    min |
|             Min cumulative indexing time across primary shards |                      |     202.422 |    min |
|          Median cumulative indexing time across primary shards |                      |     202.422 |    min |
|             Max cumulative indexing time across primary shards |                      |     202.422 |    min |
|            Cumulative indexing throttle time of primary shards |                      |           0 |    min |
|    Min cumulative indexing throttle time across primary shards |                      |           0 |    min |
| Median cumulative indexing throttle time across primary shards |                      |           0 |    min |
|    Max cumulative indexing throttle time across primary shards |                      |           0 |    min |
|                        Cumulative merge time of primary shards |                      |     109.501 |    min |
|                       Cumulative merge count of primary shards |                      |         388 |        |
|                Min cumulative merge time across primary shards |                      |     109.501 |    min |
|             Median cumulative merge time across primary shards |                      |     109.501 |    min |
|                Max cumulative merge time across primary shards |                      |     109.501 |    min |
|               Cumulative merge throttle time of primary shards |                      |     39.6005 |    min |
|       Min cumulative merge throttle time across primary shards |                      |     39.6005 |    min |
|    Median cumulative merge throttle time across primary shards |                      |     39.6005 |    min |
|       Max cumulative merge throttle time across primary shards |                      |     39.6005 |    min |
|                      Cumulative refresh time of primary shards |                      |     2.21285 |    min |
|                     Cumulative refresh count of primary shards |                      |         221 |        |
|              Min cumulative refresh time across primary shards |                      |     2.21285 |    min |
|           Median cumulative refresh time across primary shards |                      |     2.21285 |    min |
|              Max cumulative refresh time across primary shards |                      |     2.21285 |    min |
|                        Cumulative flush time of primary shards |                      |     11.7106 |    min |
|                       Cumulative flush count of primary shards |                      |         187 |        |
|                Min cumulative flush time across primary shards |                      |     11.7106 |    min |
|             Median cumulative flush time across primary shards |                      |     11.7106 |    min |
|                Max cumulative flush time across primary shards |                      |     11.7106 |    min |
|                                        Total Young Gen GC time |                      |         9.4 |      s |
|                                       Total Young Gen GC count |                      |         657 |        |
|                                          Total Old Gen GC time |                      |           0 |      s |
|                                         Total Old Gen GC count |                      |           0 |        |
|                                                     Store size |                      |      81.889 |     GB |
|                                                  Translog size |                      | 1.02445e-07 |     GB |
|                                         Heap used for segments |                      |           0 |     MB |
|                                       Heap used for doc values |                      |           0 |     MB |
|                                            Heap used for terms |                      |           0 |     MB |
|                                            Heap used for norms |                      |           0 |     MB |
|                                           Heap used for points |                      |           0 |     MB |
|                                    Heap used for stored fields |                      |           0 |     MB |
|                                                  Segment count |                      |          40 |        |
|                                       100th percentile latency |         create-index |     15.1375 |     ms |
|                                  100th percentile service time |         create-index |     15.1375 |     ms |
|                                                     error rate |         create-index |         100 |      % |
|                                       100th percentile latency | check-cluster-health |     30003.5 |     ms |
|                                  100th percentile service time | check-cluster-health |     30003.5 |     ms |
|                                                     error rate | check-cluster-health |         100 |      % |
|                                                 Min Throughput |                index |     52240.4 | docs/s |
|                                                Mean Throughput |                index |     54825.9 | docs/s |
|                                              Median Throughput |                index |     54327.3 | docs/s |
|                                                 Max Throughput |                index |     59199.1 | docs/s |
|                                        50th percentile latency |                index |     145.753 |     ms |
|                                        90th percentile latency |                index |     186.755 |     ms |
|                                        99th percentile latency |                index |     1464.82 |     ms |
|                                      99.9th percentile latency |                index |     3642.92 |     ms |
|                                     99.99th percentile latency |                index |     4719.31 |     ms |
|                                       100th percentile latency |                index |     10071.9 |     ms |
|                                   50th percentile service time |                index |     145.753 |     ms |
|                                   90th percentile service time |                index |     186.755 |     ms |
|                                   99th percentile service time |                index |     1464.82 |     ms |
|                                 99.9th percentile service time |                index |     3642.92 |     ms |
|                                99.99th percentile service time |                index |     4719.31 |     ms |
|                                  100th percentile service time |                index |     10071.9 |     ms |
|                                                     error rate |                index |           0 |      % |

Rally Result of ParserFilter

------------------------------------------------------
    _______             __   _____
   / ____(_)___  ____ _/ /  / ___/_________  ________
  / /_  / / __ \/ __ `/ /   \__ \/ ___/ __ \/ ___/ _ \
 / __/ / / / / / /_/ / /   ___/ / /__/ /_/ / /  /  __/
/_/   /_/_/ /_/\__,_/_/   /____/\___/\____/_/   \___/
------------------------------------------------------

|                                                         Metric |                 Task |       Value |   Unit |
|---------------------------------------------------------------:|---------------------:|------------:|-------:|
|                     Cumulative indexing time of primary shards |                      |     204.356 |    min |
|             Min cumulative indexing time across primary shards |                      |     204.356 |    min |
|          Median cumulative indexing time across primary shards |                      |     204.356 |    min |
|             Max cumulative indexing time across primary shards |                      |     204.356 |    min |
|            Cumulative indexing throttle time of primary shards |                      |           0 |    min |
|    Min cumulative indexing throttle time across primary shards |                      |           0 |    min |
| Median cumulative indexing throttle time across primary shards |                      |           0 |    min |
|    Max cumulative indexing throttle time across primary shards |                      |           0 |    min |
|                        Cumulative merge time of primary shards |                      |     106.225 |    min |
|                       Cumulative merge count of primary shards |                      |         397 |        |
|                Min cumulative merge time across primary shards |                      |     106.225 |    min |
|             Median cumulative merge time across primary shards |                      |     106.225 |    min |
|                Max cumulative merge time across primary shards |                      |     106.225 |    min |
|               Cumulative merge throttle time of primary shards |                      |     34.2447 |    min |
|       Min cumulative merge throttle time across primary shards |                      |     34.2447 |    min |
|    Median cumulative merge throttle time across primary shards |                      |     34.2447 |    min |
|       Max cumulative merge throttle time across primary shards |                      |     34.2447 |    min |
|                      Cumulative refresh time of primary shards |                      |     2.14308 |    min |
|                     Cumulative refresh count of primary shards |                      |         221 |        |
|              Min cumulative refresh time across primary shards |                      |     2.14308 |    min |
|           Median cumulative refresh time across primary shards |                      |     2.14308 |    min |
|              Max cumulative refresh time across primary shards |                      |     2.14308 |    min |
|                        Cumulative flush time of primary shards |                      |     11.0575 |    min |
|                       Cumulative flush count of primary shards |                      |         188 |        |
|                Min cumulative flush time across primary shards |                      |     11.0575 |    min |
|             Median cumulative flush time across primary shards |                      |     11.0575 |    min |
|                Max cumulative flush time across primary shards |                      |     11.0575 |    min |
|                                        Total Young Gen GC time |                      |       9.416 |      s |
|                                       Total Young Gen GC count |                      |         649 |        |
|                                          Total Old Gen GC time |                      |           0 |      s |
|                                         Total Old Gen GC count |                      |           0 |        |
|                                                     Store size |                      |     83.4966 |     GB |
|                                                  Translog size |                      | 1.02445e-07 |     GB |
|                                         Heap used for segments |                      |           0 |     MB |
|                                       Heap used for doc values |                      |           0 |     MB |

▽
|                                            Heap used for terms |                      |           0 |     MB |
|                                            Heap used for norms |                      |           0 |     MB |
|                                           Heap used for points |                      |           0 |     MB |
|                                    Heap used for stored fields |                      |           0 |     MB |
|                                                  Segment count |                      |          43 |        |
|                                       100th percentile latency |         create-index |     10.2571 |     ms |
|                                  100th percentile service time |         create-index |     10.2571 |     ms |
|                                                     error rate |         create-index |         100 |      % |
|                                       100th percentile latency | check-cluster-health |     30003.9 |     ms |
|                                  100th percentile service time | check-cluster-health |     30003.9 |     ms |
|                                                     error rate | check-cluster-health |         100 |      % |
|                                                 Min Throughput |                index |     56525.2 | docs/s |
|                                                Mean Throughput |                index |     58761.2 | docs/s |
|                                              Median Throughput |                index |     58217.2 | docs/s |
|                                                 Max Throughput |                index |       62062 | docs/s |
|                                        50th percentile latency |                index |     144.171 |     ms |
|                                        90th percentile latency |                index |     182.389 |     ms |
|                                        99th percentile latency |                index |      1106.4 |     ms |
|                                      99.9th percentile latency |                index |     2990.65 |     ms |
|                                     99.99th percentile latency |                index |      3797.9 |     ms |
|                                       100th percentile latency |                index |     4727.99 |     ms |
|                                   50th percentile service time |                index |     144.171 |     ms |
|                                   90th percentile service time |                index |     182.389 |     ms |
|                                   99th percentile service time |                index |      1106.4 |     ms |
|                                 99.9th percentile service time |                index |     2990.65 |     ms |
|                                99.99th percentile service time |                index |      3797.9 |     ms |
|                                  100th percentile service time |                index |     4727.99 |     ms |
|                                                     error rate |                index |           0 |      % |

elasticmachine · 2021-12-22T08:40:28Z

Pinging @elastic/es-search (Team:Search)

romseygeek

This looks great @mushao999, thanks for opening. I'd like @nik9000 to have a look as well before we merge it but LGTM in general. I left a couple of suggestions.

server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java

…urce_filtering

nik9000 · 2022-01-05T14:44:09Z

I'd like @nik9000 to have a look as well before we merge it but LGTM in general. I left a couple of suggestions.

Ah! @romseygeek this is the one I'd hope you'd review. And you already did. Cool. I'll take a look too.

nik9000 · 2022-01-05T14:50:15Z

I'm a little surprised there isn't less GC. I mean, there is a little less GC, but I'd hoped for better. I'll bet the GC from the rest of the indexing process is just swamping the gains.

server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java

nik9000 · 2022-01-05T15:00:42Z

I wonder if it's worth moving your change into XContentMapValues.filter(includes, excludes) so we get it everywhere. Presumably all the places that filter bytes should use this method if they can.

nik9000 · 2022-01-05T15:02:25Z

@elasticmachine test this please

mushao999 · 2022-01-06T11:48:11Z

I wonder if it's worth moving your change into XContentMapValues.filter(includes, excludes) so we get it everywhere. Presumably all the places that filter bytes should use this method if they can.

Applying the new filtering mechanism throughout all the filtering cases is a good idea. But I do not think XContentMapValues.filter(includes, excludes) is a good place, we will have to convert source to map before filtering which is costly and unnecessary for the new mechanism.

romseygeek · 2022-01-10T17:46:18Z

we will have to convert source to map before filtering which is costly and unnecessary for the new mechanism.

This is a good point! Can we instead create a new interface XContentFieldFilter that extends CheckedBiFunction<BytesReference, XContentType, BytesReference, IOException>, and add a static method newFilter(String[] includes, String[] excludes) that does what you've added to SourceFieldMapper? And we can then use this in other places as well - from a first look we can use it in FetchSourcePhase and ShardGetService at least. If it looks as though converting the other locations will be too much for one PR then we can break things up a bit, but ShardGetService should be simple and I think it would be helpful to have this in two places so we can check the API makes sense generally.

mushao999 · 2022-01-11T06:45:20Z

we will have to convert source to map before filtering which is costly and unnecessary for the new mechanism.

This is a good point! Can we instead create a new interface XContentFieldFilter that extends CheckedBiFunction<BytesReference, XContentType, BytesReference, IOException>, and add a static method newFilter(String[] includes, String[] excludes) that does what you've added to SourceFieldMapper? And we can then use this in other places as well - from a first look we can use it in FetchSourcePhase and ShardGetService at least. If it looks as though converting the other locations will be too much for one PR then we can break things up a bit, but ShardGetService should be simple and I think it would be helpful to have this in two places so we can check the API makes sense generally.

Thanks, this is a great suggestion. I'v added two commits.

First one introduced XContentFieldFilter and added newFieldFilter method in XContentHelper
Second one use the XContentFieldFIlter in ShardGetService.

By the way, I think XContentFieldFilter can be a member in FetchSourceContext which can avoid constructing filter for every docs, in a new PR maybe.

romseygeek

I left a couple of comments but this looks good overall. Thanks for being patient with all of our suggestions!

server/src/main/java/org/elasticsearch/common/xcontent/XContentHelper.java

romseygeek · 2022-01-11T09:13:37Z

@elasticmachine test this please

…urce_filtering

romseygeek · 2022-01-11T14:14:53Z

The build is failing on checkstyle - can you also run ./gradlew precommit and fix up any styling issues that it finds?

mushao999 · 2022-01-11T15:00:54Z

@romseygeek Thanks for you suggestion. The newFilter method has been moved to XContentFieldFilter. Test and checkStyle problems have also been fixed. Please help to invoke the test again.

server/src/main/java/org/elasticsearch/common/xcontent/XContentHelper.java

romseygeek · 2022-01-11T15:28:42Z

@elasticmachine test this please

mushao999 · 2022-01-11T15:58:14Z

@romseygeek There are still two test failed , I will try to fix them tomorrow. Thanks again.

mushao999 · 2022-01-12T08:42:24Z

@romseygeek Sorry for the test failure. Tests failed due to null value of xContentType in XContentFieldFilter#apply. I've added the null value handling in this method and run tests locally for many times.

romseygeek · 2022-01-12T10:37:50Z

@elasticmachine test this please

romseygeek

LGTM, thanks for your patience on this

This reverts commit 4560a0c. It unexpectedly caused elastic#82891.

This reverts commit 4560a0c. It unexpectedly caused #82891.

This commit introduces XContentFieldFilter, which applies field includes/excludes to XContent without having to realise the xcontent itself as a java map. SourceFieldMapper and ShardGetService are cut over to use this class

optimize source filtering in SourceFieldMapper using xcontent filteri…

14c9c7e

…ng config

elasticsearchmachine added v8.1.0 external-contributor Pull request authored by a developer outside the Elasticsearch team labels Dec 21, 2021

mushao999 changed the title ~~optimize source filtering in SourceFieldMapper using xcontent filteri…~~ optimize source filtering in SourceFieldMapper Dec 21, 2021

astefan added the :Search Foundations/Mapping Index mappings, including merging and defining field types label Dec 22, 2021

elasticmachine added the Team:Search Meta label for search team label Dec 22, 2021

romseygeek reviewed Dec 22, 2021

View reviewed changes

server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java Outdated Show resolved Hide resolved

server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java Outdated Show resolved Hide resolved

mushao999 added 2 commits December 23, 2021 14:14

update follow romseygeek's suggestions

7de88d2

Merge remote-tracking branch 'origin/master' into feature/optimize_so…

3ca7be4

…urce_filtering

nik9000 requested changes Jan 5, 2022

View reviewed changes

server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java Outdated Show resolved Hide resolved

nik9000 added the >enhancement label Jan 5, 2022

mushao999 added 2 commits January 6, 2022 11:26

format code

70a211b

use map filter if exclude has wildcard

50b406f

mushao999 added 2 commits January 11, 2022 12:01

introduce XContentFieldFilter

004e69d

use XContentFieldFilter in ShardGetService

05593c7

romseygeek reviewed Jan 11, 2022

View reviewed changes

server/src/main/java/org/elasticsearch/common/xcontent/XContentHelper.java Outdated Show resolved Hide resolved

server/src/main/java/org/elasticsearch/common/xcontent/XContentHelper.java Outdated Show resolved Hide resolved

Merge remote-tracking branch 'origin/master' into feature/optimize_so…

f96765f

…urce_filtering

mushao999 added 2 commits January 11, 2022 22:31

fix test problem

96deb00

move newFilter from XContentHelper to XContentFieldFilter

cc48d67

fix checkstyle problem

5e04845

romseygeek reviewed Jan 11, 2022

View reviewed changes

server/src/main/java/org/elasticsearch/common/xcontent/XContentHelper.java Show resolved Hide resolved

add javadoc for xContentTypeMayCompressed

1ed588f

XContentType in XContentFieldFilter may be null

872bf8a

romseygeek approved these changes Jan 12, 2022

View reviewed changes

romseygeek merged commit 4560a0c into elastic:master Jan 12, 2022

tylersmalley mentioned this pull request Jan 20, 2022

Support for superuser not having write access elastic/kibana#123338

Closed

jportner mentioned this pull request Jan 20, 2022

get/mget fails with json_generation_exception error when using source field that doesn't exist #82891

Closed

nik9000 added a commit to nik9000/elasticsearch that referenced this pull request Jan 20, 2022

Revert "Add XContentFieldFilter (elastic#81970)"

46f7063

This reverts commit 4560a0c. It unexpectedly caused elastic#82891.

nik9000 mentioned this pull request Jan 20, 2022

Revert "Add XContentFieldFilter (#81970)" #82897

Merged

elasticsearchmachine pushed a commit that referenced this pull request Jan 20, 2022

Revert "Add XContentFieldFilter (#81970)" (#82897)

8892b77

This reverts commit 4560a0c. It unexpectedly caused #82891.

mushao999 mentioned this pull request Jan 26, 2022

Fix and optimize "Add XContentFieldFilter (#81970)" #83152

Merged

Conversation

mushao999 commented Dec 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mushao999 commented Dec 21, 2021

Rally Result of MapFilter

Rally Result of ParserFilter

Uh oh!

elasticmachine commented Dec 22, 2021

Uh oh!

romseygeek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nik9000 commented Jan 5, 2022

Uh oh!

nik9000 commented Jan 5, 2022

Uh oh!

Uh oh!

nik9000 commented Jan 5, 2022

Uh oh!

nik9000 commented Jan 5, 2022

Uh oh!

mushao999 commented Jan 6, 2022

Uh oh!

romseygeek commented Jan 10, 2022

Uh oh!

mushao999 commented Jan 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

romseygeek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

romseygeek commented Jan 11, 2022

Uh oh!

romseygeek commented Jan 11, 2022

Uh oh!

mushao999 commented Jan 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

romseygeek commented Jan 11, 2022

Uh oh!

mushao999 commented Jan 11, 2022

Uh oh!

mushao999 commented Jan 12, 2022

Uh oh!

romseygeek commented Jan 12, 2022

Uh oh!

romseygeek left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mushao999 commented Dec 21, 2021 •

edited

Loading

mushao999 commented Jan 11, 2022 •

edited

Loading

mushao999 commented Jan 11, 2022 •

edited

Loading