Fixed Actor metadata is stored under 'actors||actortype' partitionkey

[markbosch]

Description

Changed that the Actor metadata is stored under correct partitionkey via 'constructCompositionKey' instead of 'calculatePartitionKey' which is used for the reminders partitions.

This results in that the Actor metadata can be found and updated when a reminder gets stored or deleted, otherwise this will create a new metadata record with incorrect metadata configuration like 'partitionCount'.

Issue reference

Please reference the issue this PR will close: 3380 (components-contrib)

Checklist

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

• Code compiles correctly
• Created/updated tests
• Unit tests passing
• End-to-end tests passing
• Extended the documentation / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]
• Specification has been updated / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]
• Provided sample for the feature / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]

[markbosch]

Small comment to elaborate on the PR:

I did some investigations regarding the issue 3380 which is reported to the compontents-contrib and I compared the behaviour in v1.10.4 (where it was working) and where it got broken (versions above that) and it turns out that when you create a reminder it will do a 'getActorMetadata' request based with a partitionKey 'constructCompositeKey("actors", actorType)', but this key is not created during the initial 'migrate' which creates the actor metadata in the fist place. During the initial creating of the actor metadata the partitionKey is based on the metadata.ID which is a GUID and passed through the transaction as a partitionKey.

This results in that the metadata can't be found when you call 'storeReminder' or 'deleteReminder' and it will create a new actor metadata record with a incorrect configuration e.g. partitionKey 0.

The fix is that it will execute two requests (transactions) instead of one with different metadata on the transactions. I can imaging this is maybe not what you want, but I don't see any way on how to distinguish between reminder partitionKeys and actor metadata partitionKeys in one transaction. Maybe somebody can point me to that?

[ItalyPaleAle]

The fix is that it will execute two requests (transactions) instead of one with different metadata on the transactions.

I don't think we can do this sadly, because at this stage it's not a single transaction, so if Dapr crashed half-way we'd have inconsistent state (or even if more than 1 Dapr sidecar is trying to write the same data)

[markbosch]

The fix is that it will execute two requests (transactions) instead of one with different metadata on the transactions.

I don't think we can do this sadly, because at this stage it's not a single transaction, so if Dapr crashed half-way we'd have inconsistent state (or even if more than 1 Dapr sidecar is trying to write the same data)

@ItalyPaleAle thanks for your reply.

Had the same thought indeed and did some digging into the state implementation of cosmos in the components-contrib.

It's unfortunately not allowed to execute a transaction with different partitionKeys, so the actor-metadata and the reminders needs to be created with a 'predictable' partitionKey which can always be reconstructed. Otherwise the getActorTypeMetadata will always fail, because that is retrieving data based on the following partitionKey: metadataPartitionKey: constructCompositeKey("actors", actorType),

So, if I change that (use the constructCompositeKey(...)) it will work, but then it makes me wonder why this databasePartitionKey is calculated?

// Get the database partition key (needed for CosmosDB)
databasePartitionKey := actorMetadata.calculateDatabasePartitionKey(stateKey)

Do you know if there is any reason why there is chosen for a GUID as a partitionKey? Or would be constructCompositeKey also be fine? Or any other thoughts on this?

Or,

Is there a way to obtain the new de facto metadataID, which is stated in the following comment:

// Also create a request to save the new metadata, so the new "metadataID" becomes the new de facto referenced list for reminders
stateOperations[len(stateOperations)-1] = r.saveActorTypeMetadataRequest(actorType, actorMetadata, stateMetadata)

so that it can be used in the getActorTypeMetadata