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

[Storage] Support Failover #4107

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"parameters": {
"subscriptionId": "{subscription-id}",
"resourceGroupName": "res4228",
"accountName": "sto2434",
"api-version": "2018-07-01",
"monitor": "true"
},
"responses": {
"200": {
"body": ""
},
"202": {
"body": ""
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"parameters": {
"subscriptionId": "{subscription-id}",
"resourceGroupName": "res4228",
"accountName": "sto2434",
"api-version": "2018-07-01",
"monitor": "true"
},
"responses": {
"200": {
"body": {
"status": "Live",
"lastSyncTime": "2017-05-24T13:25:33Z"
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,10 @@
"description": "Accepted -- Create or update request accepted; operation will complete asynchronously."
}
},
"x-ms-long-running-operation": true
"x-ms-long-running-operation": true,
"x-ms-long-running-operation-options": {
"final-state-via": "location"
}
},
"delete": {
"tags": [
Expand Down Expand Up @@ -555,6 +558,82 @@
}
}
}
},
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Storage/storageAccounts/{accountName}/failover": {
"post": {
"tags": [
"StorageAccounts"
],
"operationId": "StorageAccounts_Failover",
"description": "Failover request can be triggered for a storage account in case of availability issues. The failover occurs from the storage account's primary cluster to secondary cluster for RA-GRS accounts. The secondary cluster will become primary after failover.",
"x-ms-examples": {
"StorageAccountCreate": {
"$ref": "./examples/StorageAccountFailover.json"
}
},
"parameters": [
{
"$ref": "#/parameters/ResourceGroupName"
},
{
"$ref": "#/parameters/StorageAccountName"
},
{
"$ref": "../../../../../common-types/resource-management/v1/types.json#/parameters/ApiVersionParameter"
},
{
"$ref": "../../../../../common-types/resource-management/v1/types.json#/parameters/SubscriptionIdParameter"
}
],
"responses": {
"200": {
"description": "OK -- Returned when the storage account failover is completed, and the secondary cluster has become primary."
},
"202": {
"description": "Accepted -- Failover request accepted; operation will complete asynchronously."
}
},
"x-ms-long-running-operation": true,
"x-ms-long-running-operation-options": {
"final-state-via": "location"
}
}
},
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Storage/storageAccounts/{accountName}/lastSyncTime": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why isnt last sync time exposes as a property on the storage account itself?

Copy link
Member Author

@blueww blueww Oct 11, 2018

Choose a reason for hiding this comment

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

From the feature team:

The design of returning storage account’s properties is synchronized and RSRP returns local cache only and it does not refresh from the stamp, given the fact the all the existing properties are somewhat still and it only gets updated when the user performs some action. LST on the other hand gets computed every 2 minutes and the account row on backend get updated accordingly. It does not make sense for us to sync between RSRP and stamp every two minutes for all geo-replicated accounts, which is a very expensive operation. We rather make it on demand.

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case, it could be returned using a $expand query parameter. Only when the user explicitly specifies this, then you return this as a property on the storage account. By default, just doing get does not return it.
It really seems like a property of the account and in the current way of modeling it makes it looks like a nested resource under storageAccount which it is not.

"get": {
"tags": [
"StorageAccounts"
],
"operationId": "StorageAccounts_GetLastSyncTime",
"description": "Retrieve last sync time for his or her RA-GRS and GRS accounts.",
"x-ms-examples": {
"StorageAccountGetProperties": {
"$ref": "./examples/StorageAccountGetLastSyncTime.json"
}
},
"parameters": [
{
"$ref": "#/parameters/ResourceGroupName"
},
{
"$ref": "#/parameters/StorageAccountName"
},
{
"$ref": "../../../../../common-types/resource-management/v1/types.json#/parameters/ApiVersionParameter"
},
{
"$ref": "../../../../../common-types/resource-management/v1/types.json#/parameters/SubscriptionIdParameter"
}
],
"responses": {
"200": {
"description": "OK -- last sync time retrieved successfully for the storage account.",
"schema": {
"$ref": "#/definitions/GetLastSyncTimeResult"
}
}
}
}
}
},
"definitions": {
Expand Down Expand Up @@ -1375,8 +1454,14 @@
},
"isHnsEnabled": {
"type": "boolean",
"x-ms-client-name": "IsHnsEnabled",
"description": "Account HierarchicalNamespace enabled if sets to true."
"x-ms-client-name": "IsHnsEnabled",
"description": "Account HierarchicalNamespace enabled if sets to true."
},
"failoverInProgress": {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you make this a generic property like "status" with states like failing over etc. It will allow you reuse the same property for other status related information in future.

Copy link
Member Author

@blueww blueww Oct 11, 2018

Choose a reason for hiding this comment

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

This status is a very special status just for Get LST from secondary location status. I don't see any possibility that it can be reused by any other APIs in the recent future. So I would prefer to keep it as what it is now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok lets atleast make this a string enum. bools are less descriptive and dont allow for future expansion of states.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ravbhatnagar
Do you mean make the "failoverInProgress" a enum. Actually for this property's meaning, it almost won't have future expansion (won't add more value). It means if the failover is in progress. The value can only be "True" (means failover in progress) or null (means failover not in progress). I think a bool? can make user more easier to us the value in SDK, they only need to check the value is True, and don't need to compare the string is "True".

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, thats fine. i just wanted to note that we are strongly recommending teams against using bools. Even if its just two states, string enums work much better. Its fine if you dont want to change it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! I will take care of every bool value usage in swagger.

"type": "boolean",
"x-ms-client-name": "FailoverInProgress ",
"description": "If the failover is in progress, the value will be true, otherwise, it will be null.",
"readOnly": true
}
},
"description": "Properties of the storage account."
Expand Down Expand Up @@ -1876,6 +1961,31 @@
}
},
"description": "The List service SAS credentials operation response."
},
"GetLastSyncTimeResult": {
"properties": {
"status": {
"readOnly": true,
"type": "string",
"description": "The status of the secondary location of the Storage Account. Live: Indicates that the secondary location is active and operational; Bootstrap: Indicates initial synchronization from the primary location to the secondary location is in progress, this typically occurs when replication is first enabled; Unavailable: Indicates that the secondary location is temporarily unavailable. ",
"enum": [
"Live",
"Bootstrap",
"Unavailable"
],
"x-ms-enum": {
"name": "ReplicationStatus",
"modelAsString": true
}
},
"lastSyncTime": {
"readOnly": true,
"type": "string",
"format": "date-time",
"description": "All primary writes preceding this value are guaranteed to be replicated to secondary. Primary writes after this point in time may or may not be replicated. It is the minimum last sync time of the account’s Blob/Table/Queue/File endpoints. The value may be account’s creation time if LastSyncTime is not available. This can happen if the replication status is bootstrap or unavailable. "
}
},
"description": "The response from the get last synctime operation."
}
},
"parameters": {
Expand Down