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

[Internal] Request Hedging: Refactors HedgeRegion diagnostics field to only show successful region #4625

Conversation

NaluTripician
Copy link
Contributor

@NaluTripician NaluTripician commented Aug 2, 2024

Pull Request Template

Description

Refactors Hedge Context. Hedge Context will show what regions the SDK reaches out to.

We are also replacing Hedge Regions with Response Region, showing what region the response is coming from.

Type of change

Please delete options that are not relevant.

  • [] New feature (non-breaking change which adds functionality)

Diagnostics Pre Change

{
    "Summary": {
        "DirectCalls": {
            "(200, 0)": 2
        },
        "GatewayCalls": {
            "(200, 0)": 4,
            "(304, 0)": 1
        }
    },
    "name": "ReadItemAsync",
    "start datetime": "2024-08-02T19:17:15.724Z",
    "duration in milliseconds": 1150.7291,
    "data": {
        "Client Configuration": {
            "Client Created Time Utc": "2024-08-02T19:17:15.7212891Z",
            "MachineId": "hashedMachineName:b06bba98-6c97-3610-9de4-1b586eee7a34",
            "NumberOfClientsCreated": 2,
            "NumberOfActiveClients": 2,
            "ConnectionMode": "Direct",
            "User Agent": "cosmos-netstandard-sdk/3.42.0|2|X64|Microsoft Windows 10.0.22631|.NET 6.0.32|L|",
            "ConnectionConfig": {
                "gw": "(cps:50, urto:6, p:False, httpf: False)",
                "rntbd": "(cto: 5, icto: -1, mrpc: 30, mcpe: 65535, erd: True, pr: ReuseUnicastPort)",
                "other": "(ed:False, be:False)"
            },
            "ConsistencyConfig": "(consistency: NotSet, prgns:[Central US, North Central US], apprgn: )",
            "ProcessorCount": 12
        },
        "Hedge Regions": "(North Central US, https://dotnet-v3-multiregion-northcentralus.documents.azure.com/), (Central US, https://dotnet-v3-multiregion-centralus.documents.azure.com/)",
        "Hedge Context": "Hedged Request"
    }
 }

Diagnostics Post Change

{
    "Summary": {
        "DirectCalls": {
            "(200, 0)": 2
        },
        "GatewayCalls": {
            "(200, 0)": 4,
            "(304, 0)": 1
        }
    },
    "name": "ReadItemAsync",
    "start datetime": "2024-08-07T17:49:59.591Z",
    "duration in milliseconds": 1209.7946,
    "data": {
        "Client Configuration": {
            "Client Created Time Utc": "2024-08-07T17:49:59.5902882Z",
            "MachineId": "hashedMachineName:b06bba98-6c97-3610-9de4-1b586eee7a34",
            "NumberOfClientsCreated": 2,
            "NumberOfActiveClients": 2,
            "ConnectionMode": "Direct",
            "User Agent": "cosmos-netstandard-sdk/3.42.0|2|X64|Microsoft Windows 10.0.22631|.NET 6.0.32|L|",
            "ConnectionConfig": {
                "gw": "(cps:50, urto:6, p:False, httpf: False)",
                "rntbd": "(cto: 5, icto: -1, mrpc: 30, mcpe: 65535, erd: True, pr: ReuseUnicastPort)",
                "other": "(ed:False, be:False)"
            },
            "ConsistencyConfig": "(consistency: NotSet, prgns:[Central US, North Central US], apprgn: )",
            "ProcessorCount": 12
        },
        "Hedge Context": "Central US, North Central US",
        "Response Region": "North Central US"
    }
}

@NaluTripician NaluTripician self-assigned this Aug 2, 2024
@NaluTripician NaluTripician added the auto-merge Enables automation to merge PRs label Aug 2, 2024
@NaluTripician NaluTripician removed the auto-merge Enables automation to merge PRs label Aug 2, 2024
@NaluTripician NaluTripician added the auto-merge Enables automation to merge PRs label Aug 5, 2024
@kirankumarkolli
Copy link
Member

In Description: Exclude region is showing only after update

@kirankumarkolli
Copy link
Member

    "Hedge Context": "[Central US, Hedged Request Sent],[North Central US, Hedged Request Successful]",
    "Response Region": "North Central US"

Thinking loud: "Response Region" already covers which Task completed first.
In "Hedged Context" per region state is Task status so are they redundant?

Update from offline:

  • Hedged Context will only carry the regions

@NaluTripician
Copy link
Contributor Author

In Description: Exclude region is showing only after update

Logs were from before remove exclude regions PR was merged. Updated to reflect current state.

@microsoft-github-policy-service microsoft-github-policy-service bot merged commit c2043ae into master Aug 9, 2024
21 checks passed
@microsoft-github-policy-service microsoft-github-policy-service bot deleted the users/nalutripician/hedgingDiagnosticsHedgeRegion branch August 9, 2024 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge Enables automation to merge PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants