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.

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

feat: add support for Row Affinity app profiles #2341

Merged
merged 10 commits into from
Dec 4, 2024
Merged

feat: add support for Row Affinity app profiles #2341

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b18d5ca
Create of methods, add unit tests
djyau Sep 16, 2024
e3be203
Add unit tests for BigtableInstanceAdminClientTests
djyau Sep 16, 2024
8019c29
Add tests for Create and Update App Profile
djyau Sep 16, 2024
d10c5cf
Add integration test
djyau Sep 16, 2024
047b046
Rename ofWithRowAffinity to withRowAffinity
djyau Oct 1, 2024
464d1bb
Merge branch 'main' of sso://cloud-bigtable-internal/java-bigtable in…
djyau Oct 1, 2024
4f197d4
Run mvn com.coveo:fmt-maven-plugin:format
djyau Oct 11, 2024
c6f6a78
Remove extra test for cluster IDs - one is enough
djyau Oct 11, 2024
fecd6ea
Add unit test for row affinity with set of strings
djyau Oct 11, 2024
7ba35f8
Run formatter
djyau Oct 14, 2024
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
30 changes: 30 additions & 0 deletions ...le-cloud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/models/AppProfile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ private AppProfile(@Nonnull com.google.bigtable.admin.v2.AppProfile proto) {
@SuppressWarnings("WeakerAccess")
public RoutingPolicy getPolicy() {
if (proto.hasMultiClusterRoutingUseAny()) {
if (proto.getMultiClusterRoutingUseAny().hasRowAffinity()) {
return MultiClusterRoutingPolicy.ofWithRowAffinity(
ImmutableSet.copyOf(proto.getMultiClusterRoutingUseAny().getClusterIdsList()));
}
return MultiClusterRoutingPolicy.of(
ImmutableSet.copyOf(proto.getMultiClusterRoutingUseAny().getClusterIdsList()));
} else if (proto.hasSingleClusterRouting()) {
Expand Down Expand Up @@ -267,6 +271,32 @@ public static MultiClusterRoutingPolicy of(Set<String> clusterIds) {
MultiClusterRoutingUseAny.newBuilder().addAllClusterIds(clusterIds).build());
}

/** Creates a new instance of {@link MultiClusterRoutingPolicy}. */
public static MultiClusterRoutingPolicy ofWithRowAffinity() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think withRowAffinity is a bit less akward

Copy link
Contributor

Choose a reason for hiding this comment

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

Also can you describe what the difference between this variant and withRowAffinity(String... clusterIds)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also is it possible to update an app profile to change cluster ids? or to change the routing away from row affinity?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also do you need to update the mask?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also can you describe what the difference between this variant and withRowAffinity(String... clusterIds)

Yes - Row affinity, when enabled, routes single row key requests deterministically to the same cluster each time to improve read-your-writes consistency.

When row affinity is enabled with a cluster group (with cluster IDs specified), then when the user adds a new cluster to their instance, it does not affect which clusters each row key routes to. It is a way to protect read-your-writes consistency from being impacted negatively from new clusters as well as from attempts to delete clusters (since you cannot delete a cluster if its in the cluster group). Without the cluster group, when a user adds a new cluster, row keys will undergo a shuffling of which cluster they route to, which temporarily breaks read-your-writes consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also do you need to update the mask?

I don't think so, but correct me if I am wrong: row affinity is a subfield/submessage of MultiClusterRoutingUseAny (routing policy), and we already have existing methods for updating the routing policy with the appropriate mask: https://github.com/googleapis/java-bigtable/blob/main/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/models/UpdateAppProfileRequest.java#L106-L125

Also is it possible to update an app profile to change cluster ids? or to change the routing away from row affinity?

It should be possible to do both of these. The user should call UpdateAppProfileRequest::SetRoutingPolicy and supply a routing policy with new cluster ids or with row affinity absent

return new MultiClusterRoutingPolicy(MultiClusterRoutingUseAny.newBuilder()
.setRowAffinity(MultiClusterRoutingUseAny.RowAffinity.getDefaultInstance()).build());
}

/**
* Creates a new instance of {@link MultiClusterRoutingPolicy} with row affinity enabled and specified cluster ids to
* route to.
*/
public static MultiClusterRoutingPolicy ofWithRowAffinity(String... clusterIds) {
return ofWithRowAffinity(ImmutableSet.copyOf(clusterIds));
}

/**
* Creates a new instance of {@link MultiClusterRoutingPolicy} with specified cluster ids to
* route to.
*/
public static MultiClusterRoutingPolicy ofWithRowAffinity(Set<String> clusterIds) {
return new MultiClusterRoutingPolicy(
MultiClusterRoutingUseAny.newBuilder()
.addAllClusterIds(clusterIds)
.setRowAffinity(MultiClusterRoutingUseAny.RowAffinity.getDefaultInstance())
.build());
}

/*
* Returns the set of clusters to route to. The order is ignored; clusters will be
* tried in order of distance. If empty, all clusters are eligible.
Expand Down
129 changes: 129 additions & 0 deletions ...le/src/test/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClientTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1034,6 +1034,135 @@ public void testCreateAppProfileAddPriority() {
assertThat(actualResult).isEqualTo(AppProfile.fromProto(expectedResponse));
}

@Test
public void testCreateAppProfileAddRowAffinity() {
// Setup
Mockito.when(mockStub.createAppProfileCallable()).thenReturn(mockCreateAppProfileCallable);

com.google.bigtable.admin.v2.CreateAppProfileRequest expectedRequest =
com.google.bigtable.admin.v2.CreateAppProfileRequest.newBuilder()
.setParent(NameUtil.formatInstanceName(PROJECT_ID, INSTANCE_ID))
.setAppProfileId(APP_PROFILE_ID)
.setAppProfile(
com.google.bigtable.admin.v2.AppProfile.newBuilder()
.setDescription("my description")
.setMultiClusterRoutingUseAny(
com.google.bigtable.admin.v2.AppProfile.MultiClusterRoutingUseAny
.newBuilder()
.setRowAffinity(com.google.bigtable.admin.v2.AppProfile.MultiClusterRoutingUseAny.RowAffinity.getDefaultInstance())))
.build();

com.google.bigtable.admin.v2.AppProfile expectedResponse =
com.google.bigtable.admin.v2.AppProfile.newBuilder()
.setName(APP_PROFILE_NAME)
.setDescription("my description")
.setMultiClusterRoutingUseAny(
com.google.bigtable.admin.v2.AppProfile.MultiClusterRoutingUseAny.newBuilder()
.setRowAffinity(com.google.bigtable.admin.v2.AppProfile.MultiClusterRoutingUseAny.RowAffinity.getDefaultInstance()))
.build();

Mockito.when(mockCreateAppProfileCallable.futureCall(expectedRequest))
.thenReturn(ApiFutures.immediateFuture(expectedResponse));

// Execute
AppProfile actualResult =
adminClient.createAppProfile(
CreateAppProfileRequest.of(INSTANCE_ID, APP_PROFILE_ID)
.setDescription("my description")
.setRoutingPolicy(MultiClusterRoutingPolicy.ofWithRowAffinity()));

// Verify
assertThat(actualResult).isEqualTo(AppProfile.fromProto(expectedResponse));
}

@Test
public void testCreateAppProfileAddRowAffinityAddMultipleClusterIds() {
// Setup
Mockito.when(mockStub.createAppProfileCallable()).thenReturn(mockCreateAppProfileCallable);

com.google.bigtable.admin.v2.CreateAppProfileRequest expectedRequest =
com.google.bigtable.admin.v2.CreateAppProfileRequest.newBuilder()
.setParent(NameUtil.formatInstanceName(PROJECT_ID, INSTANCE_ID))
.setAppProfileId(APP_PROFILE_ID)
.setAppProfile(
com.google.bigtable.admin.v2.AppProfile.newBuilder()
.setDescription("my description")
.setMultiClusterRoutingUseAny(
com.google.bigtable.admin.v2.AppProfile.MultiClusterRoutingUseAny
.newBuilder()
.addClusterIds("cluster-id-1")
.addClusterIds("cluster-id-2")
.setRowAffinity(com.google.bigtable.admin.v2.AppProfile.MultiClusterRoutingUseAny.RowAffinity.getDefaultInstance())))
.build();

com.google.bigtable.admin.v2.AppProfile expectedResponse =
com.google.bigtable.admin.v2.AppProfile.newBuilder()
.setName(APP_PROFILE_NAME)
.setDescription("my description")
.setMultiClusterRoutingUseAny(
com.google.bigtable.admin.v2.AppProfile.MultiClusterRoutingUseAny.newBuilder()
.addClusterIds("cluster-id-1")
.addClusterIds("cluster-id-2")
.setRowAffinity(com.google.bigtable.admin.v2.AppProfile.MultiClusterRoutingUseAny.RowAffinity.getDefaultInstance()))
.build();

Mockito.when(mockCreateAppProfileCallable.futureCall(expectedRequest))
.thenReturn(ApiFutures.immediateFuture(expectedResponse));

// Execute
AppProfile actualResult =
adminClient.createAppProfile(
CreateAppProfileRequest.of(INSTANCE_ID, APP_PROFILE_ID)
.setDescription("my description")
.setRoutingPolicy(MultiClusterRoutingPolicy.ofWithRowAffinity("cluster-id-1", "cluster-id-2")));

// Verify
assertThat(actualResult).isEqualTo(AppProfile.fromProto(expectedResponse));
}

@Test
public void testCreateAppProfileAddRowAffinityAddMultipleClusterIdsWithList() {
// Setup
Mockito.when(mockStub.createAppProfileCallable()).thenReturn(mockCreateAppProfileCallable);

com.google.bigtable.admin.v2.CreateAppProfileRequest expectedRequest =
com.google.bigtable.admin.v2.CreateAppProfileRequest.newBuilder()
.setParent(NameUtil.formatInstanceName(PROJECT_ID, INSTANCE_ID))
.setAppProfileId(APP_PROFILE_ID)
.setAppProfile(
com.google.bigtable.admin.v2.AppProfile.newBuilder()
.setDescription("my description")
.setMultiClusterRoutingUseAny(
com.google.bigtable.admin.v2.AppProfile.MultiClusterRoutingUseAny
.newBuilder()
.addAllClusterIds(ImmutableList.of("cluster-id-1", "cluster-id-2"))
.setRowAffinity(com.google.bigtable.admin.v2.AppProfile.MultiClusterRoutingUseAny.RowAffinity.getDefaultInstance())))
.build();

com.google.bigtable.admin.v2.AppProfile expectedResponse =
com.google.bigtable.admin.v2.AppProfile.newBuilder()
.setName(APP_PROFILE_NAME)
.setDescription("my description")
.setMultiClusterRoutingUseAny(
com.google.bigtable.admin.v2.AppProfile.MultiClusterRoutingUseAny.newBuilder()
.addAllClusterIds(ImmutableList.of("cluster-id-1", "cluster-id-2"))
.setRowAffinity(com.google.bigtable.admin.v2.AppProfile.MultiClusterRoutingUseAny.RowAffinity.getDefaultInstance()))
.build();

Mockito.when(mockCreateAppProfileCallable.futureCall(expectedRequest))
.thenReturn(ApiFutures.immediateFuture(expectedResponse));

// Execute
AppProfile actualResult =
adminClient.createAppProfile(
CreateAppProfileRequest.of(INSTANCE_ID, APP_PROFILE_ID)
.setDescription("my description")
.setRoutingPolicy(MultiClusterRoutingPolicy.ofWithRowAffinity("cluster-id-1", "cluster-id-2")));

// Verify
assertThat(actualResult).isEqualTo(AppProfile.fromProto(expectedResponse));
}

@Test
public void testGetAppProfile() {
// Setup
Expand Down
34 changes: 34 additions & 0 deletions ...le/src/test/java/com/google/cloud/bigtable/admin/v2/it/BigtableInstanceAdminClientIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,40 @@ public void appProfileTestDataBoost() {
}
}

@Test
public void appProfileTestRowAffinity() {
String newInstanceId = prefixGenerator.newPrefix();
String newClusterId = newInstanceId + "-c1";
String newClusterId2 = newInstanceId + "-c2";

client.createInstance(
CreateInstanceRequest.of(newInstanceId)
.addCluster(newClusterId, testEnvRule.env().getPrimaryZone(), 1, StorageType.SSD)
.addCluster(newClusterId2, testEnvRule.env().getSecondaryZone(), 1, StorageType.SSD)
.setDisplayName("Row-Affinity-Instance-Test")
.addLabel("state", "readytodelete")
.setType(Type.PRODUCTION));

try {
assertThat(client.exists(newInstanceId)).isTrue();

String testAppProfile = prefixGenerator.newPrefix();

CreateAppProfileRequest request =
CreateAppProfileRequest.of(newInstanceId, testAppProfile)
.setRoutingPolicy(AppProfile.MultiClusterRoutingPolicy.ofWithRowAffinity(newClusterId, newClusterId2))
.setDescription("row affinity app profile");

AppProfile newlyCreateAppProfile = client.createAppProfile(request);
AppProfile.RoutingPolicy routingPolicy = newlyCreateAppProfile.getPolicy();
assertThat(routingPolicy).isEqualTo(AppProfile.MultiClusterRoutingPolicy.ofWithRowAffinity(newClusterId, newClusterId2));
} finally {
if (client.exists(newInstanceId)) {
client.deleteInstance(newInstanceId);
}
}
}

@Test
public void iamUpdateTest() {
Policy policy = client.getIamPolicy(instanceId);
Expand Down
44 changes: 44 additions & 0 deletions ...loud-bigtable/src/test/java/com/google/cloud/bigtable/admin/v2/models/AppProfileTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,4 +291,48 @@ public void testFromProtoWithDataBoostIsolation() {
AppProfile.DataBoostIsolationReadOnlyPolicy.of(
AppProfile.ComputeBillingOwner.UNSPECIFIED));
}

@Test
public void testFromProtoWithRowAffinityNoClusterGroup() {
AppProfile profile =
AppProfile.fromProto(
com.google.bigtable.admin.v2.AppProfile.newBuilder()
.setName(AppProfileName.of("my-project", "my-instance", "my-profile").toString())
.setDescription("my description")
.setMultiClusterRoutingUseAny(
com.google.bigtable.admin.v2.AppProfile.MultiClusterRoutingUseAny.newBuilder()
.setRowAffinity(com.google.bigtable.admin.v2.AppProfile.MultiClusterRoutingUseAny.RowAffinity.getDefaultInstance())
.build())
.setEtag("my-etag")
.build());

assertThat(profile.getInstanceId()).isEqualTo("my-instance");
assertThat(profile.getId()).isEqualTo("my-profile");
assertThat(profile.getDescription()).isEqualTo("my description");
System.out.println(profile.getPolicy());
System.out.println(AppProfile.MultiClusterRoutingPolicy.ofWithRowAffinity());
assertThat(profile.getPolicy())
.isEqualTo(AppProfile.MultiClusterRoutingPolicy.ofWithRowAffinity());
}
@Test
public void testFromProtoWithRowAffinityClusterGroup() {
AppProfile profile =
AppProfile.fromProto(
com.google.bigtable.admin.v2.AppProfile.newBuilder()
.setName(AppProfileName.of("my-project", "my-instance", "my-profile").toString())
.setDescription("my description")
.setMultiClusterRoutingUseAny(
com.google.bigtable.admin.v2.AppProfile.MultiClusterRoutingUseAny.newBuilder()
.addAllClusterIds(ImmutableList.of("cluster-id-1", "cluster-id-2"))
.setRowAffinity(com.google.bigtable.admin.v2.AppProfile.MultiClusterRoutingUseAny.RowAffinity.getDefaultInstance())
.build())
.setEtag("my-etag")
.build());

assertThat(profile.getInstanceId()).isEqualTo("my-instance");
assertThat(profile.getId()).isEqualTo("my-profile");
assertThat(profile.getDescription()).isEqualTo("my description");
assertThat(profile.getPolicy())
.isEqualTo(AppProfile.MultiClusterRoutingPolicy.ofWithRowAffinity("cluster-id-1", "cluster-id-2"));
}
}
10 changes: 10 additions & 0 deletions .../src/test/java/com/google/cloud/bigtable/admin/v2/models/CreateAppProfileRequestTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,14 @@ public void testDataBoostIsolationReadOnly() {
.setComputeBillingOwner(DataBoostIsolationReadOnly.ComputeBillingOwner.HOST_PAYS)
.build());
}

@Test
public void testRowAffinity() {
CreateAppProfileRequest wrapper =
CreateAppProfileRequest.of("my-instance", "my-profile")
.setRoutingPolicy(MultiClusterRoutingPolicy.ofWithRowAffinity());

assertThat(wrapper.toProto("my-project").getAppProfile().getMultiClusterRoutingUseAny())
.isEqualTo(MultiClusterRoutingUseAny.newBuilder().setRowAffinity(MultiClusterRoutingUseAny.RowAffinity.getDefaultInstance()).build());
}
}
23 changes: 23 additions & 0 deletions .../src/test/java/com/google/cloud/bigtable/admin/v2/models/UpdateAppProfileRequestTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,4 +146,27 @@ public void testUpdateExistingDataBoostIsolationReadOnly() {
.setUpdateMask(FieldMask.newBuilder().addPaths("data_boost_isolation_read_only"))
.build());
}

@Test
public void testUpdateRowAffinity() {
com.google.bigtable.admin.v2.AppProfile existingProto =
com.google.bigtable.admin.v2.AppProfile.newBuilder()
.setName("projects/my-project/instances/my-instance/appProfiles/my-profile")
.setEtag("my-etag")
.setDescription("description")
.setMultiClusterRoutingUseAny(MultiClusterRoutingUseAny.getDefaultInstance())
.build();

AppProfile existingWrapper = AppProfile.fromProto(existingProto);

UpdateAppProfileRequest updateWrapper =
UpdateAppProfileRequest.of(existingWrapper).setRoutingPolicy(AppProfile.MultiClusterRoutingPolicy.ofWithRowAffinity());

assertThat(updateWrapper.toProto("my-project"))
.isEqualTo(
com.google.bigtable.admin.v2.UpdateAppProfileRequest.newBuilder()
.setAppProfile(existingProto.toBuilder().setMultiClusterRoutingUseAny(MultiClusterRoutingUseAny.newBuilder().setRowAffinity(MultiClusterRoutingUseAny.RowAffinity.getDefaultInstance())))
.setUpdateMask(FieldMask.newBuilder().addPaths("multi_cluster_routing_use_any"))
.build());
}
}