CosmosDBState - Add, as option, the partitionKey in QueryMethod #3227

luigirende · 2023-11-15T16:30:44Z

Opportunity to add the partitionKey field in the metadata of the Query request (how it's done in the Set and Get method).
If the user adds the partitionKey in the query method, the database before filtering all the items that belongs to the same partitionKey value, used in the request; after applies the query defined in the request. In this case the cross partition will be disabled, so the scan of the item that the database will do in the container will be only for a subset of item, improving the performance

Description

Please explain the changes you've made

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #[issue number]

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

Code compiles correctly
Created/updated tests
Extended the documentation / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]

berndverst · 2023-11-28T03:22:36Z

state/azure/cosmosdb/cosmosdb.go

@@ -92,8 +92,11 @@ func (p crossPartitionQueryPolicy) Do(req *policy.Request) (*http.Response, erro
 	// Check if we're performing a query
 	// In that case, remove the partitionkey header and enable cross-partition queries
 	if strings.ToLower(raw.Header.Get("x-ms-documentdb-query")) == "true" {
-		raw.Header.Add("x-ms-documentdb-query-enablecrosspartition", "true")
-		raw.Header.Del("x-ms-documentdb-partitionkey")
+		raw.Header.Add("x-ms-documentdb-query-enablecrosspartition", "True")


probably should be lowercase true the way it was before

Move this into the if-clause you added. This header should only be added when the partition key is fake. You do not want to run expensive cross partition queries otherwise.

state/feature.go

state/azure/cosmosdb/cosmosdb.go

berndverst

Generally the idea is right, but some small correction on the details. When to add headers etc

Also please do not define the new feature key / capability. That is only intended for features which we (at least at some point in time) intend to support in all state stores.

luigirende · 2023-11-28T14:28:06Z

tests/conformance/state/state.go

+		// Test for CosmosDB (filter test with partitionOnly) this query doesn't use the cross partition ( value from 2 different partitionKey %s-struct and %s-struct-2)
+		{
+			query: `
+			{
+				"filter": {
+					"OR": [
+						{
+							"EQ": {"message": "` + key + `test"}
+						},
+						{
+							"IN": {"message": ["test` + key + `", "dummy"]}
+						}
+					]
+				}
+			}
+			`,
+			metadata: map[string]string{
+				"partitionKey": fmt.Sprintf("%s-struct-2", key),
+			},
+			partitionOnly: true,
+			// The same query from previous test but return only item having the same partitionKey value (%s-struct-2) given in the metadata
+			results: []state.QueryItem{
+				{
+					Key:  fmt.Sprintf("%s-struct-2", key),
+					Data: []byte(fmt.Sprintf(`{"message":"%stest"}`, key)),
+				},
+			},
+		},


@berndverst I added PartitionKey as feature for doing this test; this scenario will be ok only for CosmosDB, because for the others states will return 2 items instead of 1.

I see. Not ideal - but I can't at the moment think of a better way to do this either.

tests/conformance/state/state.go

berndverst

I finally got around to merging your query operators PR.

Could you resolve the merge conflict here now and then we'll get this one merged also? :)

luigirende · 2023-12-07T18:08:08Z

@berndverst done!

berndverst · 2023-12-11T20:34:27Z

@luigirende I'll merge this. Can you please make sure to take ownership of this docs issue and make the docs PR? Otherwise nobody will know about this.

dapr/docs#3897

Use /assign on that issue.

berndverst · 2023-12-11T22:57:44Z

/ok-to-test

dapr-bot · 2023-12-11T22:58:10Z

Components certification test

🔗 Link to Action run

Commit ref: 45f3b74

❌ Some certification tests failed

These tests failed:

pubsub.gcp.pubsub

dapr-bot · 2023-12-11T22:58:10Z

Complete Build Matrix

The build status is currently not updated here. Please visit the action run below directly.

🔗 Link to Action run

Commit ref: 45f3b74

dapr-bot · 2023-12-11T22:58:12Z

Components conformance test

🔗 Link to Action run

Commit ref: 45f3b74

❌ Some conformance tests failed

These tests failed:

state.azure.cosmosdb
state.postgresql.azure

berndverst

The CosmosDB conformance test is failing.

luigirende · 2023-12-12T12:19:12Z

The CosmosDB conformance test is failing.

Hi @berndverst

I think there has been an issue with azure tests. In fact fails also the azure postgresql tes and in this PR I haven't done any change on postgresql state.
Looking at the test log seems the dbs (cosmosdb and postgresql) weren't empty, when run the tests, in fact, the query find other keys store previously.

    	Error Trace:	/home/runner/work/components-contrib/components-contrib/tests/conformance/state/state.go:478
    	Error:      	Not equal: 
    	            	expected: "1fa86a779a7d49f0ba1a15fe7ce75f01-struct-operations"
    	            	actual  : "429bcb9db8d04c8da420d686492f0239-struct-operations"

Can you do a check or test again the PR?

berndverst · 2023-12-18T20:39:21Z

The CosmosDB conformance test is failing.

Hi @berndverst

I think there has been an issue with azure tests. In fact fails also the azure postgresql tes and in this PR I haven't done any change on postgresql state. Looking at the test log seems the dbs (cosmosdb and postgresql) weren't empty, when run the tests, in fact, the query find other keys store previously.
    	Error Trace:	/home/runner/work/components-contrib/components-contrib/tests/conformance/state/state.go:478
    	Error:      	Not equal: 
    	            	expected: "1fa86a779a7d49f0ba1a15fe7ce75f01-struct-operations"
    	            	actual  : "429bcb9db8d04c8da420d686492f0239-struct-operations"
Can you do a check or test again the PR?

@luigirende I think this because of your other PR which is now merged, but you also must keep in mind that multiple tests might be running in parallel. Your test must work in that situation!

When the test starts it uses a unique prefix for the test run. So you need to write your test such that it will only look for the results with this particular prefix.

Also keep in mind you still have a conflict in state/feature.go.

luigirende · 2023-12-19T11:35:37Z

@berndverst merged the conflict in state/feature.go and fix the issue in the conformance tests

…data request for the Query Method (uniform with Set and Get). Disabling the cross partition Signed-off-by: luigirende <[email protected]>

Signed-off-by: Luigi Rende <[email protected]>

Signed-off-by: Bernd Verst <[email protected]>

Signed-off-by: Luigi Rende <[email protected]>

berndverst · 2024-01-17T17:24:54Z

/ok-to-test

dapr-bot · 2024-01-17T17:25:54Z

Complete Build Matrix

The build status is currently not updated here. Please visit the action run below directly.

🔗 Link to Action run

Commit ref: c1f2bdf

dapr-bot · 2024-01-17T17:25:56Z

Components certification test

🔗 Link to Action run

Commit ref: c1f2bdf

❌ Some certification tests failed

These tests failed:

bindings.azure.servicebusqueues
pubsub.mqtt3

dapr-bot · 2024-01-17T17:25:56Z

Components conformance test

🔗 Link to Action run

Commit ref: c1f2bdf

❌ Some conformance tests failed

These tests failed:

pubsub.kafka-confluent

berndverst · 2024-01-25T00:38:01Z

/ok-to-test

dapr-bot · 2024-01-25T00:38:22Z

Complete Build Matrix

The build status is currently not updated here. Please visit the action run below directly.

🔗 Link to Action run

Commit ref: 46a5d05

dapr-bot · 2024-01-25T00:38:24Z

Components conformance test

🔗 Link to Action run

Commit ref: 46a5d05

✅ All conformance tests passed

All tests have reported a successful status

dapr-bot · 2024-01-25T00:38:29Z

Components certification test

🔗 Link to Action run

Commit ref: 46a5d05

❌ Some certification tests failed

These tests failed:

bindings.azure.eventhubs
bindings.azure.servicebusqueues
bindings.azure.storagequeues
pubsub.azure.eventhubs
pubsub.azure.servicebus.topics
pubsub.mqtt3

luigirende requested review from a team as code owners November 15, 2023 16:30

luigirende force-pushed the cosmosdb-paritionkey-query branch from 9ee628a to 16ed7d1 Compare November 22, 2023 11:42

This was referenced Nov 22, 2023

Add new filters type in the query of the state component #3218

Merged

Feature Request: CosmosDB State Store Query to support partitioned queries #3244

Open

berndverst reviewed Nov 28, 2023

View reviewed changes

state/feature.go Show resolved Hide resolved

berndverst reviewed Nov 28, 2023

View reviewed changes

state/azure/cosmosdb/cosmosdb.go Show resolved Hide resolved

berndverst reviewed Nov 28, 2023

View reviewed changes

state/azure/cosmosdb/cosmosdb.go Show resolved Hide resolved

berndverst requested changes Nov 28, 2023

View reviewed changes

luigirende commented Nov 28, 2023

View reviewed changes

tests/conformance/state/state.go Show resolved Hide resolved

luigirende force-pushed the cosmosdb-paritionkey-query branch 4 times, most recently from e3be222 to 6f57467 Compare December 5, 2023 17:03

berndverst requested changes Dec 7, 2023

View reviewed changes

luigirende force-pushed the cosmosdb-paritionkey-query branch from 6f57467 to 2a7ff4d Compare December 7, 2023 18:07

berndverst mentioned this pull request Dec 11, 2023

Document CosmosDB Partitioned Queries dapr/docs#3897

Open

berndverst approved these changes Dec 11, 2023

View reviewed changes

berndverst requested changes Dec 11, 2023

View reviewed changes

luigirende force-pushed the cosmosdb-paritionkey-query branch from 45f3b74 to a2dc965 Compare December 19, 2023 11:29

luigirende force-pushed the cosmosdb-paritionkey-query branch 3 times, most recently from a2dc965 to 5ca1b69 Compare January 9, 2024 13:54

luigirende and others added 4 commits January 11, 2024 15:25

CosmosDBState - Opportunity to add the partitionKey value in the meta…

fb83a05

…data request for the Query Method (uniform with Set and Get). Disabling the cross partition Signed-off-by: luigirende <[email protected]>

CosmosDB - improvement of the filter for cross partition

cac2965

Signed-off-by: Luigi Rende <[email protected]>

gofumpt

08a91cf

Signed-off-by: Bernd Verst <[email protected]>

Fix tests in the conformance state

a806f06

Signed-off-by: Luigi Rende <[email protected]>

luigirende force-pushed the cosmosdb-paritionkey-query branch from 5ca1b69 to a806f06 Compare January 11, 2024 14:25

Merge branch 'main' into cosmosdb-paritionkey-query

c1f2bdf

Merge branch 'main' into cosmosdb-paritionkey-query

46a5d05

berndverst approved these changes Jan 25, 2024

View reviewed changes

berndverst merged commit 120a649 into dapr:main Jan 25, 2024
88 of 89 checks passed

berndverst added this to the v1.13 milestone Jan 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CosmosDBState - Add, as option, the partitionKey in QueryMethod #3227

CosmosDBState - Add, as option, the partitionKey in QueryMethod #3227

luigirende commented Nov 15, 2023 •

edited

Loading

berndverst Nov 28, 2023 •

edited

Loading

berndverst Nov 28, 2023

berndverst left a comment

luigirende Nov 28, 2023

berndverst Dec 11, 2023

berndverst left a comment

luigirende commented Dec 7, 2023

berndverst commented Dec 11, 2023

berndverst commented Dec 11, 2023

dapr-bot commented Dec 11, 2023 •

edited

Loading

dapr-bot commented Dec 11, 2023

dapr-bot commented Dec 11, 2023 •

edited

Loading

berndverst left a comment

luigirende commented Dec 12, 2023 •

edited

Loading

berndverst commented Dec 18, 2023

luigirende commented Dec 19, 2023

berndverst commented Jan 17, 2024

dapr-bot commented Jan 17, 2024

dapr-bot commented Jan 17, 2024 •

edited

Loading

dapr-bot commented Jan 17, 2024 •

edited

Loading

berndverst commented Jan 25, 2024

dapr-bot commented Jan 25, 2024

dapr-bot commented Jan 25, 2024 •

edited

Loading

dapr-bot commented Jan 25, 2024 •

edited

Loading

CosmosDBState - Add, as option, the partitionKey in QueryMethod #3227

CosmosDBState - Add, as option, the partitionKey in QueryMethod #3227

Conversation

luigirende commented Nov 15, 2023 • edited Loading

Description

Issue reference

Checklist

berndverst Nov 28, 2023 • edited Loading

Choose a reason for hiding this comment

berndverst Nov 28, 2023

Choose a reason for hiding this comment

berndverst left a comment

Choose a reason for hiding this comment

luigirende Nov 28, 2023

Choose a reason for hiding this comment

berndverst Dec 11, 2023

Choose a reason for hiding this comment

berndverst left a comment

Choose a reason for hiding this comment

luigirende commented Dec 7, 2023

berndverst commented Dec 11, 2023

berndverst commented Dec 11, 2023

dapr-bot commented Dec 11, 2023 • edited Loading

Components certification test

❌ Some certification tests failed

dapr-bot commented Dec 11, 2023

Complete Build Matrix

dapr-bot commented Dec 11, 2023 • edited Loading

Components conformance test

❌ Some conformance tests failed

berndverst left a comment

Choose a reason for hiding this comment

luigirende commented Dec 12, 2023 • edited Loading

berndverst commented Dec 18, 2023

luigirende commented Dec 19, 2023

berndverst commented Jan 17, 2024

dapr-bot commented Jan 17, 2024

Complete Build Matrix

dapr-bot commented Jan 17, 2024 • edited Loading

Components certification test

❌ Some certification tests failed

dapr-bot commented Jan 17, 2024 • edited Loading

Components conformance test

❌ Some conformance tests failed

berndverst commented Jan 25, 2024

dapr-bot commented Jan 25, 2024

Complete Build Matrix

dapr-bot commented Jan 25, 2024 • edited Loading

Components conformance test

✅ All conformance tests passed

dapr-bot commented Jan 25, 2024 • edited Loading

Components certification test

❌ Some certification tests failed

luigirende commented Nov 15, 2023 •

edited

Loading

berndverst Nov 28, 2023 •

edited

Loading

dapr-bot commented Dec 11, 2023 •

edited

Loading

dapr-bot commented Dec 11, 2023 •

edited

Loading

luigirende commented Dec 12, 2023 •

edited

Loading

dapr-bot commented Jan 17, 2024 •

edited

Loading

dapr-bot commented Jan 17, 2024 •

edited

Loading

dapr-bot commented Jan 25, 2024 •

edited

Loading

dapr-bot commented Jan 25, 2024 •

edited

Loading