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

CosmosQueryRequestOptions excludes old FeedOptions #33

Closed
davetheunissen opened this issue Jan 26, 2019 · 10 comments
Closed

CosmosQueryRequestOptions excludes old FeedOptions #33

davetheunissen opened this issue Jan 26, 2019 · 10 comments
Assignees
Labels
feature-request New feature or request

Comments

@davetheunissen
Copy link

davetheunissen commented Jan 26, 2019

Is your feature request related to a problem? Please describe.
Not sure if this is a regression bug or feature request but the PopulateQueryMetrics feed option is not exposed as a member of the CosmosQueryRequestOptions class.

In fact there are a number of feed options which have been left out between the current client and this one. Have these options been left off for a reason or are they configurable else where?

Comparison between FeedOptions & CosmosQueryRequestOptions

Property Status
ConsistencyLevel Supported
EnableCrossPartitionQuery Internally Set (no public access)
EnableLowPrecisionOrderBy Supported
EnableScanInQuery Supported
JsonSerializerSettings Not Supported
MaxBufferedItemCount Supported
MaxDegreeOfParallelism Supported (called MaxConcurrency and no longer request option)
MaxItemCount Supported (no longer request option)
PartitionKey Supported
PartitionKeyRangeId Not Supported
PopulateQueryMetrics Not Supported
RequestContinuation Not Supported
ResponseContinuationTokenLimitInKb Supported
SessionToken Supported

Describe the solution you'd like
Are there any objections to adding PopulateQueryMetrics as an option to CosmosQueryRequestOptions? I've tested and confirmed that CosmosQueryResponse<T> includes the metrics if set. This setting is really useful for performance tuning etc.

@davetheunissen
Copy link
Author

If there are no objections to adding PopulateQueryMetrics I will submit a PR with the change.

@kirankumarkolli
Copy link
Member

@j82w do you see any issues with including it?

@kirankumarkolli kirankumarkolli added the feature-request New feature or request label Jan 30, 2019
@j82w
Copy link
Contributor

j82w commented Jan 30, 2019

I don't see an issue with including it for now, but this will likely have some breaking changes when the diagnostics are updated.

@j82w
Copy link
Contributor

j82w commented May 17, 2019

We are still working on the v3 diagnostics design. I will create a github issue once we have a design. We want query and point operations to have the same story. In v2 they are completely different which doesn't provide the best user experience.

These properties have been updated.

Property Status
EnableCrossPartitionQuery Internally Set (no public access). Automatically set when uses overload with max concurrency.
JsonSerializerSettings Supported via CosmosJsonSerializer
PartitionKeyRangeId No plans to support.
PopulateQueryMetrics Planned
RequestContinuation Renamed to ContinuationToken

@martinsmith123456
Copy link

Is this something that will be resolved before GA release?

In the meantime I guess a workaround would be using a handler to add the "x-ms-documentdb-populatequerymetrics" header to the request and read the "x-ms-documentdb-query-metrics" from the Response.

@RobertDougan
Copy link

Hi, is there an ETA on the PopulateQueryMetrics property?

@nh43de
Copy link

nh43de commented Aug 27, 2019

Why are some of these properties no longer supported? We're migrating to the V3 API and confused why some of these properties have been removed from public access.

@j82w
Copy link
Contributor

j82w commented Aug 27, 2019

@RobertDougan the PR is out to add QueryMetrics support. Hopefully we will have a release in next 2 to 3 weeks.

@nh43de the only option that I'm aware of is the PartitionKeyRangeId and the query metrics. Query Metrics will be fixed in this PR. The reason PartitionKeyRangeId was removed is it's more of an internal implementation detail. It allows a small performance gain, but it's rather complex with lots of corner cases that don't show up until it's in production. It was decided it was better to remove the property since it caused so many issues with so little gain. Is there a specific property you need that isn't supported?

@nh43de
Copy link

nh43de commented Aug 27, 2019

Hey @j82w thanks for looking into this - we have some code that uses EnableCrossPartitionQuery in production currently.

@j82w
Copy link
Contributor

j82w commented Aug 28, 2019

@nh43de EnableCrossPartitionQuery is enabled by default in v3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants