Skip to content

Add force_synthetic_source to GET#87536

Merged
nik9000 merged 2 commits intoelastic:masterfrom
nik9000:synthetic_force_get2
Jun 9, 2022
Merged

Add force_synthetic_source to GET#87536
nik9000 merged 2 commits intoelastic:masterfrom
nik9000:synthetic_force_get2

Conversation

@nik9000
Copy link
Copy Markdown
Member

@nik9000 nik9000 commented Jun 8, 2022

This adds the option to force synthetic source to the GET API. See
#87068 for more discussion on why you'd want to do that - the short
version is to get an upper bound on the performance cost of using
synthetic source in GET.

This adds the option to force synthetic source to the GET API. See
 elastic#87068 for more discussion on why you'd want to do that - the short
version is to get an upper bound on the performance cost of using
synthetic source in GET.
@nik9000 nik9000 requested a review from romseygeek June 8, 2022 18:25
@elasticmachine elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jun 8, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

@nik9000 nik9000 mentioned this pull request Jun 8, 2022
50 tasks
@sethmlarson sethmlarson added the Team:Clients Meta label for clients team label Jun 8, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/clients-team (Team:Clients)

Copy link
Copy Markdown
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!

request.versionType(),
request.fetchSourceContext()
request.fetchSourceContext(),
request.isForceSyntheticSource()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This was making me think that maybe we should add forceSynthetic as a member on FetchSourceContext, but it looks as though that is used in loads of other places so it would end up being a much bigger change.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah. I thought about it and didn't like.

item.version(),
item.versionType(),
item.fetchSourceContext(),
false
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This will be enabled in a follow-up, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It'll be request.isForceSyntheticSource() I think.

@nik9000 nik9000 merged commit a37edb7 into elastic:master Jun 9, 2022
@javanna javanna added the :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. label Jun 24, 2022
@elasticmachine elasticmachine added the Team:Distributed Meta label for distributed team. label Jun 24, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >non-issue :StorageEngine/TSDB You know, for Metrics Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:Clients Meta label for clients team Team:Distributed Meta label for distributed team. v8.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants