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

Replace Endpoints with Regional Endpoints #39390

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

tvaron3
Copy link
Member

@tvaron3 tvaron3 commented Jan 24, 2025

Description

The service will now start returning two endpoints for a region. A GetDatabaseAccount call used to get the following uri for a region test-account-name-westus.cosmos.documents.com. Now, it will randomly send variations of this endpoint. Ex: test-account-name-westus-1.cosmos.documents.com, test-account-name-westus-2.cosmos.documents.com. This is to improve availability in scenarios when gateway goes down.

Implementation

Location cache will now have a new RegionalEndpoint object that will have a current and previous. The idea is the previous can be used in certain scenarios to retry.

Pseudocode of new current and previous logic

request in progress
on current:
    success:
        no op
    failure:
        use previous:
 
on previous:
    success:
        temp = current
        current = previous
        previous = temp 
    failure:
        refresh the cache:
            if (current != new value):
                previous = current:
            current = new value
 
on database account refresh - every 5 mins:
    initial:
        current = new value
        previous = None
    next:
        if current != new value
            previous = current
        current = new value

Testing

@tvaron3 tvaron3 requested review from annatisch and a team as code owners January 24, 2025 17:20
@azure-sdk
Copy link
Collaborator

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-cosmos

@@ -2038,7 +2038,8 @@ def PatchItem(
if options is None:
options = {}

headers = base.GetHeaders(self, self.default_headers, "patch", path, document_id, resource_type, options)
headers = base.GetHeaders(self, self.default_headers, "patch", path, document_id, resource_type,
Copy link
Member

Choose a reason for hiding this comment

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

just curious - why all small letters (patch) when the OpType is defined with capital letters here. Don't we have to do case-insensitive comaprison due to it everywehre -

class _OperationType:
"""Represents the type of the operation"""
Create: Literal["Create"] = "Create"
Delete: Literal["Delete"] = "Delete"
ExecuteJavaScript: Literal["ExecuteJavaScript"] = "ExecuteJavaScript"
Head: Literal["Head"] = "Head"
HeadFeed: Literal["HeadFeed"] = "HeadFeed"
Patch: Literal["Patch"] = "Patch"
Query: Literal["Query"] = "Query"
QueryPlan: Literal["QueryPlan"] = "QueryPlan"
Read: Literal["Read"] = "Read"
ReadFeed: Literal["ReadFeed"] = "ReadFeed"
Recreate: Literal["Recreate"] = "Recreate"
Replace: Literal["Replace"] = "Replace"
SqlQuery: Literal["SqlQuery"] = "SqlQuery"
Update: Literal["Update"] = "Update"
Upsert: Literal["Upsert"] = "Upsert"
Batch: Literal["Batch"] = "Batch"

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

except Exception as e:
raise e

for new_location in new_locations:
Copy link
Member Author

@tvaron3 tvaron3 Jan 25, 2025

Choose a reason for hiding this comment

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

Use get() instead of location[""]

@tvaron3
Copy link
Member Author

tvaron3 commented Jan 27, 2025

for example, all of these connection errors should be handled properly and can be safely retried in my opinion (more research required) https://docs.aiohttp.org/en/stable/client_reference.html#connection-errors

Further client errors by aio http package -> https://docs.aiohttp.org/en/stable/client_reference.html#client-exceptions
Client Reference — aiohttp 3.11.11 documentation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

6 participants