Skip to content

Schema change to add DecompressedLength column to Resource table #5247

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.

Sign up for GitHub

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

Open
rajithaalurims wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Schema change to add DecompressedLength column to Resource table #5247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion global.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"sdk": {
"version": "9.0.305"
"version": "9.0.306"
}
}
458 changes: 458 additions & 0 deletions src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/101.diff.sql
View file
Open in desktop

Large diffs are not rendered by default.

6,406 changes: 6,406 additions & 0 deletions src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Migrations/101.sql
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/Microsoft.Health.Fhir.SqlServer/Features/Schema/SchemaVersion.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,5 +110,6 @@ public enum SchemaVersion
V98 = 98,
V99 = 99,
V100 = 100,
V101 = 101,
}
}
3 changes: 2 additions & 1 deletion src/Microsoft.Health.Fhir.SqlServer/Features/Schema/SchemaVersionConstants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace Microsoft.Health.Fhir.SqlServer.Features.Schema
public static class SchemaVersionConstants
{
public const int Min = (int)SchemaVersion.V94;
public const int Max = (int)SchemaVersion.V100;
public const int Max = (int)SchemaVersion.V101;
public const int MinForUpgrade = (int)SchemaVersion.V94; // this is used for upgrade tests only
public const int SearchParameterStatusSchemaVersion = (int)SchemaVersion.V6;
public const int SupportForReferencesWithMissingTypeVersion = (int)SchemaVersion.V7;
Expand Down Expand Up @@ -37,6 +37,7 @@ public static class SchemaVersionConstants
public const int SearchParameterOptimisticConcurrency = (int)SchemaVersion.V95;
public const int SearchParameterMaxLastUpdatedStoredProcedure = (int)SchemaVersion.V96;
public const int SearchParameterLastUpdatedIndex = (int)SchemaVersion.V98;
public const int DecompressedLength = (int)SchemaVersion.V101;

// It is currently used in Azure Healthcare APIs.
public const int ParameterizedRemovePartitionFromResourceChangesVersion = (int)SchemaVersion.V21;
Expand Down
28 changes: 25 additions & 3 deletions src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Sprocs/MergeResources.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ CREATE PROCEDURE dbo.MergeResources
,@TokenQuantityCompositeSearchParams dbo.TokenQuantityCompositeSearchParamList READONLY
,@TokenStringCompositeSearchParams dbo.TokenStringCompositeSearchParamList READONLY
,@TokenNumberNumberCompositeSearchParams dbo.TokenNumberNumberCompositeSearchParamList READONLY
,@DecompressedOverridesJson NVARCHAR(MAX) = NULL
Copy link
Contributor

@SergeyGaluzo SergeyGaluzo Nov 24, 2025

Choose a reason for hiding this comment

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

Parsing JSON on each merge call sounds expensive. Instead, we should add decompressed length to ResourceList table type. There should be multiple iterations to get compatibility and final clean state.

Copy link
Contributor Author

@rajithaalurims rajithaalurims Nov 25, 2025

Choose a reason for hiding this comment

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

I was debating between changing ResourceList table type but as you mentioned that needs multiple iterations of code release to get final clean slate.
From what I have researched additional cost for parsing takes around few hundred milli seconds for rows around 10000.
I will discuss offline with you about which option to go with

Copy link
Contributor

@SergeyGaluzo SergeyGaluzo Nov 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe. But imagine that we start adding extra JSON vars for each new column we add... This looks like antipattern. Also, there are extra joins to @Resources, and they are extra cost too... All of these are just not applicable to the case of table type change. BTW You should get fully functioning code on first iteration...

AS
set nocount on
DECLARE @st datetime = getUTCdate()
Expand Down Expand Up @@ -56,6 +57,23 @@ BEGIN TRY

DECLARE @PreviousSurrogateIds AS TABLE (TypeId smallint NOT NULL, SurrogateId bigint NOT NULL PRIMARY KEY (TypeId, SurrogateId), KeepHistory bit)

DECLARE @DecompressedOverrides TABLE
(
DCResourceSurrogateId BIGINT PRIMARY KEY,
DecompressedLength DECIMAL(36,18) NULL
);

IF @DecompressedOverridesJson IS NOT NULL
BEGIN
INSERT INTO @DecompressedOverrides(DCResourceSurrogateId, DecompressedLength)
SELECT ResourceSurrogateId, DecompressedLength
FROM OPENJSON(@DecompressedOverridesJson)
WITH (
ResourceSurrogateId BIGINT '$.ResourceSurrogateId',
DecompressedLength DECIMAL(36,18) '$.DecompressedLength'
);
END

-- perform retry check in transaction to hold locks
IF @InitialTranCount = 0
BEGIN
Expand Down Expand Up @@ -119,6 +137,7 @@ BEGIN TRY
,RawResource = 0xF -- "invisible" value
,SearchParamHash = NULL
,HistoryTransactionId = @TransactionId
,DecompressedLength = 0
WHERE EXISTS (SELECT * FROM @PreviousSurrogateIds WHERE TypeId = ResourceTypeId AND SurrogateId = ResourceSurrogateId AND KeepHistory = 0)
ELSE
DELETE FROM dbo.Resource WHERE EXISTS (SELECT * FROM @PreviousSurrogateIds WHERE TypeId = ResourceTypeId AND SurrogateId = ResourceSurrogateId AND KeepHistory = 0)
Expand Down Expand Up @@ -160,9 +179,12 @@ BEGIN TRY
END

INSERT INTO dbo.Resource
( ResourceTypeId, ResourceId, Version, IsHistory, ResourceSurrogateId, IsDeleted, RequestMethod, RawResource, IsRawResourceMetaSet, SearchParamHash, TransactionId )
SELECT ResourceTypeId, ResourceId, Version, IsHistory, ResourceSurrogateId, IsDeleted, RequestMethod, RawResource, IsRawResourceMetaSet, SearchParamHash, @TransactionId
FROM @Resources
( ResourceTypeId, ResourceId, Version, IsHistory, ResourceSurrogateId, IsDeleted, RequestMethod, RawResource, IsRawResourceMetaSet, SearchParamHash, TransactionId, DecompressedLength )
SELECT ResourceTypeId, ResourceId, Version, IsHistory, ResourceSurrogateId, IsDeleted, RequestMethod, RawResource, IsRawResourceMetaSet, SearchParamHash, @TransactionId, OV.DecompressedLength
FROM @Resources AS R
LEFT JOIN @DecompressedOverrides AS OV
ON OV.DCResourceSurrogateId = R.ResourceSurrogateId;

SET @AffectedRows += @@rowcount

INSERT INTO dbo.ResourceWriteClaim
Expand Down
6 changes: 4 additions & 2 deletions src/Microsoft.Health.Fhir.SqlServer/Features/Schema/Sql/Tables/Resource.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ CREATE TABLE dbo.CurrentResource -- This is replaced by view CurrentResource
IsRawResourceMetaSet bit NOT NULL,
SearchParamHash varchar(64) NULL,
TransactionId bigint NULL,
HistoryTransactionId bigint NULL
HistoryTransactionId bigint NULL,
DecompressedLength decimal(36,18) NULL
Copy link
Contributor

@SergeyGaluzo SergeyGaluzo Nov 24, 2025

Choose a reason for hiding this comment

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

Why it cannot be int?

Copy link
Contributor Author

@rajithaalurims rajithaalurims Nov 25, 2025

Choose a reason for hiding this comment

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

I should have used INT. I will change

)
GO
DROP TABLE dbo.CurrentResource
Expand All @@ -32,7 +33,8 @@ CREATE TABLE dbo.Resource
IsRawResourceMetaSet bit NOT NULL DEFAULT 0,
SearchParamHash varchar(64) NULL,
TransactionId bigint NULL, -- used for main CRUD operation
HistoryTransactionId bigint NULL -- used by CRUD operation that moved resource version in invisible state
HistoryTransactionId bigint NULL, -- used by CRUD operation that moved resource version in invisible state
DecompressedLength decimal(36,18) NULL

CONSTRAINT PKC_Resource PRIMARY KEY CLUSTERED (ResourceTypeId, ResourceSurrogateId) WITH (DATA_COMPRESSION = PAGE) ON PartitionScheme_ResourceTypeId(ResourceTypeId),
CONSTRAINT CH_Resource_RawResource_Length CHECK (RawResource > 0x0)
Expand Down
18 changes: 18 additions & 0 deletions src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
using Microsoft.Health.Fhir.Core.Features.Persistence.Orchestration;
using Microsoft.Health.Fhir.Core.Features.Search;
using Microsoft.Health.Fhir.Core.Models;
using Microsoft.Health.Fhir.SqlServer.Features.Schema;
using Microsoft.Health.Fhir.SqlServer.Features.Schema.Model;
using Microsoft.Health.Fhir.SqlServer.Features.Storage.TvpRowGeneration;
using Microsoft.Health.Fhir.SqlServer.Features.Storage.TvpRowGeneration.Merge;
Expand Down Expand Up @@ -748,6 +749,12 @@ internal async Task MergeResourcesWrapperAsync(long transactionId, bool singleTr
new TokenQuantityCompositeSearchParamListTableValuedParameterDefinition("@TokenQuantityCompositeSearchParams").AddParameter(cmd.Parameters, new TokenQuantityCompositeSearchParamListRowGenerator(_model, new TokenSearchParamListRowGenerator(_model, _searchParameterTypeMap), new QuantitySearchParamListRowGenerator(_model, _searchParameterTypeMap), _searchParameterTypeMap).GenerateRows(mergeWrappers));
new TokenStringCompositeSearchParamListTableValuedParameterDefinition("@TokenStringCompositeSearchParams").AddParameter(cmd.Parameters, new TokenStringCompositeSearchParamListRowGenerator(_model, new TokenSearchParamListRowGenerator(_model, _searchParameterTypeMap), new StringSearchParamListRowGenerator(_model, _searchParameterTypeMap), _searchParameterTypeMap).GenerateRows(mergeWrappers));
new TokenNumberNumberCompositeSearchParamListTableValuedParameterDefinition("@TokenNumberNumberCompositeSearchParams").AddParameter(cmd.Parameters, new TokenNumberNumberCompositeSearchParamListRowGenerator(_model, new TokenSearchParamListRowGenerator(_model, _searchParameterTypeMap), new NumberSearchParamListRowGenerator(_model, _searchParameterTypeMap), _searchParameterTypeMap).GenerateRows(mergeWrappers));

if (_schemaInformation.Current >= SchemaVersionConstants.DecompressedLength)
{
cmd.Parameters.AddWithValue("@DecompressedOverridesJson", SqlServerFhirDataStore.GenerateResourceMetricsJson(mergeWrappers));
}

var commandTimeout = 300 + (int)(3600.0 / 10000 * (timeoutRetries + 1) * mergeWrappers.Count);
cmd.CommandTimeout = commandTimeout;

Expand Down Expand Up @@ -1045,5 +1052,16 @@ public async Task<ResourceWrapper> UpdateSearchParameterIndicesAsync(ResourceWra
{
return await Task.FromResult((int?)null);
}

private static string GenerateResourceMetricsJson(IReadOnlyList<MergeResourceWrapper> mergeWrappers)
{
var metrics = mergeWrappers.Select(wrapper => new
{
ResourceSurrogateId = wrapper.ResourceWrapper.ResourceSurrogateId,
DecompressedLength = System.Text.Encoding.UTF8.GetByteCount(wrapper.ResourceWrapper.RawResource.Data),
}).ToArray();

return System.Text.Json.JsonSerializer.Serialize(metrics);
}
}
}
2 changes: 1 addition & 1 deletion src/Microsoft.Health.Fhir.SqlServer/Microsoft.Health.Fhir.SqlServer.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<!-- Properties used by sql task to generate full script -->
<PropertyGroup>
<LatestSchemaVersion>100</LatestSchemaVersion>
<LatestSchemaVersion>101</LatestSchemaVersion>
<GeneratedFullScriptPath>Features\Schema\Migrations\$(LatestSchemaVersion).sql</GeneratedFullScriptPath>
</PropertyGroup>

Expand Down
Loading