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

Fix timing bug with DFS profiling #92421

Merged
merged 4 commits into from
Jan 3, 2023

Conversation

benwtrent
Copy link
Member

Introduced in: #90536

Profiling for DFS has had its timing numbers looking weird, additionally, it would trigger some assertion failures because timer.start() was called without a stop() in between.

The key issue was around query weight creation. Weight creation could be called recursively, thus calling start on the timer more than once before calling stop.

@benwtrent benwtrent added >bug :Search/Search Search-related issues that do not fall into other categories v8.6.1 v8.7.0 labels Dec 16, 2022
@elasticsearchmachine
Copy link
Collaborator

Hi @benwtrent, I've created a changelog YAML for you.

@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Search Meta label for search team label Dec 16, 2022
@benwtrent
Copy link
Member Author

Here is what hybrid-search profile looks like for DFS:

{
"profile": {
        "shards": [
            {
                "id": "[JtUlpv_9S4yYUpkimBBw_g][image-index][0]",
                "dfs": {
                    "statistics": {
                        "type": "statistics",
                        "description": "collect term statistics",
                        "time_in_nanos": 359750,
                        "breakdown": {
                            "term_statistics": 2833,
                            "collection_statistics": 42916,
                            "collection_statistics_count": 2,
                            "create_weight": 350917,
                            "term_statistics_count": 1,
                            "rewrite_count": 0,
                            "create_weight_count": 1,
                            "rewrite": 0
                        }
                    },
                    "knn": {
                        "query": [
                            {
                                "type": "BoostQuery",
                                "description": "(DocAndScore[50])^0.1",
                                "time_in_nanos": 329293,
                                "breakdown": {
                                    "set_min_competitive_score_count": 0,
                                    "match_count": 0,
                                    "shallow_advance_count": 0,
                                    "set_min_competitive_score": 0,
                                    "next_doc": 1709,
                                    "match": 0,
                                    "next_doc_count": 4,
                                    "score_count": 4,
                                    "compute_max_score_count": 0,
                                    "compute_max_score": 0,
                                    "advance": 308750,
                                    "advance_count": 3,
                                    "count_weight_count": 0,
                                    "score": 1376,
                                    "build_scorer_count": 6,
                                    "create_weight": 2084,
                                    "shallow_advance": 0,
                                    "count_weight": 0,
                                    "create_weight_count": 1,
                                    "build_scorer": 15374
                                }
                            }
                        ],
                        "rewrite_time": 317792,
                        "collector": [
                            {
                                "name": "SimpleTopScoreDocCollector",
                                "reason": "search_top_hits",
                                "time_in_nanos": 21626
                            }
                        ]
                    }
                },
                "searches": [
                    {
                        "query": [
                            {
                                "type": "BooleanQuery",
                                "description": "ScoreAndDocQuery (title:mountain title:lake)^0.9",
                                "time_in_nanos": 827749,
                                "breakdown": {
                                    "set_min_competitive_score_count": 0,
                                    "match_count": 1,
                                    "shallow_advance_count": 0,
                                    "set_min_competitive_score": 0,
                                    "next_doc": 24666,
                                    "match": 583,
                                    "next_doc_count": 4,
                                    "score_count": 4,
                                    "compute_max_score_count": 0,
                                    "compute_max_score": 0,
                                    "advance": 46250,
                                    "advance_count": 3,
                                    "count_weight_count": 0,
                                    "score": 10666,
                                    "build_scorer_count": 6,
                                    "create_weight": 346459,
                                    "shallow_advance": 0,
                                    "count_weight": 0,
                                    "create_weight_count": 1,
                                    "build_scorer": 399125
                                },
                                "children": [
                                    {
                                        "type": "KnnScoreDocQuery",
                                        "description": "ScoreAndDocQuery",
                                        "time_in_nanos": 41460,
                                        "breakdown": {
                                            "set_min_competitive_score_count": 0,
                                            "match_count": 0,
                                            "shallow_advance_count": 4,
                                            "set_min_competitive_score": 0,
                                            "next_doc": 1751,
                                            "match": 0,
                                            "next_doc_count": 3,
                                            "score_count": 4,
                                            "compute_max_score_count": 4,
                                            "compute_max_score": 2958,
                                            "advance": 6084,
                                            "advance_count": 4,
                                            "count_weight_count": 0,
                                            "score": 1750,
                                            "build_scorer_count": 9,
                                            "create_weight": 2500,
                                            "shallow_advance": 4750,
                                            "count_weight": 0,
                                            "create_weight_count": 1,
                                            "build_scorer": 21667
                                        }
                                    },
                                    {
                                        "type": "BoostQuery",
                                        "description": "(title:mountain title:lake)^0.9",
                                        "time_in_nanos": 536040,
                                        "breakdown": {
                                            "set_min_competitive_score_count": 0,
                                            "match_count": 0,
                                            "shallow_advance_count": 4,
                                            "set_min_competitive_score": 0,
                                            "next_doc": 0,
                                            "match": 0,
                                            "next_doc_count": 0,
                                            "score_count": 1,
                                            "compute_max_score_count": 4,
                                            "compute_max_score": 16083,
                                            "advance": 5583,
                                            "advance_count": 2,
                                            "count_weight_count": 0,
                                            "score": 4334,
                                            "build_scorer_count": 5,
                                            "create_weight": 322667,
                                            "shallow_advance": 7208,
                                            "count_weight": 0,
                                            "create_weight_count": 1,
                                            "build_scorer": 180165
                                        },
                                        "children": [
                                            {
                                                "type": "TermQuery",
                                                "description": "title:mountain",
                                                "time_in_nanos": 188875,
                                                "breakdown": {
                                                    "set_min_competitive_score_count": 0,
                                                    "match_count": 0,
                                                    "shallow_advance_count": 0,
                                                    "set_min_competitive_score": 0,
                                                    "next_doc": 0,
                                                    "match": 0,
                                                    "next_doc_count": 0,
                                                    "score_count": 0,
                                                    "compute_max_score_count": 0,
                                                    "compute_max_score": 0,
                                                    "advance": 0,
                                                    "advance_count": 0,
                                                    "count_weight_count": 0,
                                                    "score": 0,
                                                    "build_scorer_count": 3,
                                                    "create_weight": 150625,
                                                    "shallow_advance": 0,
                                                    "count_weight": 0,
                                                    "create_weight_count": 1,
                                                    "build_scorer": 38250
                                                }
                                            },
                                            {
                                                "type": "TermQuery",
                                                "description": "title:lake",
                                                "time_in_nanos": 283167,
                                                "breakdown": {
                                                    "set_min_competitive_score_count": 0,
                                                    "match_count": 0,
                                                    "shallow_advance_count": 4,
                                                    "set_min_competitive_score": 0,
                                                    "next_doc": 0,
                                                    "match": 0,
                                                    "next_doc_count": 0,
                                                    "score_count": 1,
                                                    "compute_max_score_count": 4,
                                                    "compute_max_score": 14042,
                                                    "advance": 4292,
                                                    "advance_count": 2,
                                                    "count_weight_count": 0,
                                                    "score": 3792,
                                                    "build_scorer_count": 5,
                                                    "create_weight": 152041,
                                                    "shallow_advance": 5126,
                                                    "count_weight": 0,
                                                    "create_weight_count": 1,
                                                    "build_scorer": 103874
                                                }
                                            }
                                        ]
                                    }
                                ]
                            }
                        ],
                        "rewrite_time": 190207,
                        "collector": [
                            {
                                "name": "SimpleTopScoreDocCollector",
                                "reason": "search_top_hits",
                                "time_in_nanos": 30917
                            }
                        ]
                    }
                ],
                "aggregations": [],
                "fetch": {
                    "type": "fetch",
                    "description": "",
                    "time_in_nanos": 556500,
                    "breakdown": {
                        "load_stored_fields": 204959,
                        "load_source": 31585,
                        "load_stored_fields_count": 4,
                        "next_reader_count": 3,
                        "load_source_count": 4,
                        "next_reader": 63584
                    },
                    "debug": {
                        "stored_fields": [
                            "_id",
                            "_routing",
                            "_source"
                        ]
                    },
                    "children": [
                        {
                            "type": "FetchSourcePhase",
                            "description": "",
                            "time_in_nanos": 21584,
                            "breakdown": {
                                "process_count": 4,
                                "process": 20583,
                                "next_reader": 1001,
                                "next_reader_count": 3
                            },
                            "debug": {
                                "fast_path": 4
                            }
                        },
                        {
                            "type": "StoredFieldsPhase",
                            "description": "",
                            "time_in_nanos": 15376,
                            "breakdown": {
                                "process_count": 4,
                                "process": 13624,
                                "next_reader": 1752,
                                "next_reader_count": 3
                            }
                        }
                    ]
                }
            }
        ]
    }
}

@benwtrent
Copy link
Member Author

@elasticmachine update branch

@benwtrent
Copy link
Member Author

@elasticmachine update branch

Copy link
Contributor

@romseygeek romseygeek left a comment

Choose a reason for hiding this comment

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

LGTM

@benwtrent benwtrent added the auto-backport-and-merge Automatically create backport pull requests and merge when ready label Jan 3, 2023
@benwtrent benwtrent merged commit 1262c50 into elastic:main Jan 3, 2023
@benwtrent benwtrent deleted the bugfix/dfs-profiling branch January 3, 2023 14:16
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.6

benwtrent added a commit to benwtrent/elasticsearch that referenced this pull request Jan 3, 2023
Introduced in: elastic#90536

Profiling for DFS has had its timing numbers looking weird, additionally, it would trigger some assertion failures because `timer.start()` was called without a `stop()` in between.

The key issue was around query `weight` creation. `Weight` creation could be called recursively, thus calling `start` on the timer more than once before calling stop.
elasticsearchmachine pushed a commit that referenced this pull request Jan 3, 2023
Introduced in: #90536

Profiling for DFS has had its timing numbers looking weird, additionally, it would trigger some assertion failures because `timer.start()` was called without a `stop()` in between.

The key issue was around query `weight` creation. `Weight` creation could be called recursively, thus calling `start` on the timer more than once before calling stop.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport-and-merge Automatically create backport pull requests and merge when ready >bug :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v8.6.1 v8.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants