database: Merge Cosmos DB containers #1215
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mbarnes
requested review from
bennerv,
SudoBrendan,
mociarain,
venkateshsredhat,
Nanyte25,
kostola,
geoberle,
janboll,
jmelis,
jonathan34c,
UlrichSchlueter,
weinong,
whober0521,
tony-schndr and
jfchevrette
as code owners
|
Please rebase pull request. |
mbarnes
force-pushed
the
merge-cosmos-db-containers
branch
from
1dab915 to
ccd20c3
Compare
|
Please rebase pull request. |
mbarnes
force-pushed
the
merge-cosmos-db-containers
branch
from
ccd20c3 to
e636adf
Compare
mbarnes
requested review from
machi1990,
miguelsorianod,
JameelB and
jharrington22
as code owners
added 3 commits
February 3, 2025 13:59
In anticipation of the Cosmos DB containers being merged and partitioned by subscription ID, this commit fully rewrites how the backend finds and processes operation documents in Cosmos DB. Instead of querying for all operation documents across all Azure subscriptions and processing them serially, the backend now uses a goroutine-based "worker pool" where each worker is responsible for processing operation documents within a single subscription/ (soon-to-be) Cosmos DB partition. This dramatically increases the parallelism within the backend without significantly changing the Cluster Service interaction logic. The polling intervals and worker pool size within the backend may require further tuning.
Operation documents have a limited time-to-live and are never explicitly deleted.
added 12 commits
February 3, 2025 13:59
Frontend refactoring. The rationale for this will become clear in subsequent commits.
DBClient refactoring. The rationale for this will become clear in subsequent commits.
DBClient refactoring. The rationale for this will become clear in subsequent commits.
The system-defined fields in BaseDocument are not used outside the database package. Make it explicit by not exporting the type.
Give the field names a prefix to avoid naming conflicts.
This type should not be accessible outside the database package.
Creates a partition key from an Azure subscription ID. This will eventually be called from outside the database package.
This is a major breaking change to our Cosmos DB documents.
typedDocument standardizes the structure of documents as follows:
{
"id": <item-id>
"partitionKey": <partition-key>
"resourceType": <azure-resource-type>
"properties": {
...
}
"_rid": <system-generated>
"_self": <system-generated>
"_etag": <system-generated>
"_ts": <system-generated>
}
A key insight is none of the system-generated fields are used outside
of the database package. Therefore the data that the frontend/backend
components care about can be encapsulated in a "properties" value,
and the other document types no longer need to embed baseDocument.
DocumentProperties is an interface for types that can be plugged
into the "properties" field, like ResourceDocument. It requires a
GetValidTypes() method that returns a set of valid values for the
"resourceType" field, used for validation when [un]marshalling.
One case worth noting is SubscriptionDocument was mostly already
following this format, with its "subscription" field defined as a
arm.Subscription struct. Instead, arm.Subscription now implements
the DocumentProperties interface and SubscriptionDocument is gone.
Also worth mentioning is DBClientIterator has been redefined as a
iter.Seq2 instead of a iter.Seq. Effectively this means the Items
method now returns the Cosmos DB item ID separately from the item
(document) itself.
To illustrate:
old style: for doc := range iterator.Items()
new style: for id, doc := range iterator.Items()
Apologies for the size of this commit, but the changes here are
all intertwined.
This parameter is not used yet, this is just to get some API changes out of the way. The parameter type is azcosmos.PartitionKey instead of string for type safety. If the partition key and item ID parameters were both strings, there would be a risk of callers mixing up the arguments.
Discard the 'Operations' and 'Subscriptions' containers. This data now lives in the 'Resources' container. Doing this enables the use of transactional batch operations when multiple Cosmos DB items of different types need to be created or updated together.
mbarnes
force-pushed
the
merge-cosmos-db-containers
branch
from
e636adf to
8a36e1e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does
(This pull request builds upon #1203. Please review that one first.)
This pull request contains all the breaking changes related to ARO-14170 - Merge Cosmos DB containers.
🚨 Give the ARO-HCP team a heads up before merging this! 🚨
In addition to merging the "resources", "operations", and "subscriptions" Cosmos DB containers, this pull request also changes the structure of all Cosmos DB documents in this merged container.
A new
typedDocumentstruct standardizes the structure of documents as follows:A key insight is none of the system-generated fields are used outside of the database package. Therefore the data that the frontend/backend components care about can be encapsulated in a
"properties"value, and the other document types no longer need to embedbaseDocument.DocumentPropertiesis an interface for types that can be plugged into the"properties"field, likeResourceDocument. The interface requires aGetValidTypes()method that returns a set of valid values for the"resourceType"field, used for validation when [un]marshalling.One case worth noting is
SubscriptionDocumentwas mostly already following this format, with its top-level"subscription"field defined as aarm.Subscriptionstruct:With this PR,
arm.Subscriptionnow implements theDocumentPropertiesinterface andSubscriptionDocumentis gone.Also worth mentioning is
DBClientIteratorItemhas been redefined as a iter.Seq2 instead of a iter.Seq. Effectively this means theDBClientIterator.Itemsmethod now returns the Cosmos DB item ID separately from the item (document) itself.To illustrate:
Jira: ARO-14170 - Merge Cosmos DB containers
Link to demo recording:
Special notes for your reviewer