Skip to content

Commit

Permalink
Bigtable: table model improvements (#3640)
Browse files Browse the repository at this point in the history 
* Flatten cluster
* expose column families as list only
* use relative ids for tables & families
  • Loading branch information
@igorbernstein2 @pongad
igorbernstein2 authored and pongad committed Sep 6, 2018
1 parent 47bd674 commit 3cefb4f
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 312 deletions.
76 changes: 0 additions & 76 deletions ...-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/ClusterState.java
View file

This file was deleted.

7 changes: 3 additions & 4 deletions ...-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/ColumnFamily.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@

/** Wrapper for {@link ColumnFamily} protocol buffer object */
public final class ColumnFamily {
// TODO(igorbernstein2): rename this to `name`
private final String id;
private final GCRule rule;

Expand All @@ -44,21 +43,21 @@ private ColumnFamily(String id, GCRule rule) {
}

/**
* Gets the columnfamily name
* Gets the column family's id.
*/
public String getId() {
return id;
}

/**
* Get's the GCRule configured for the columnfamily
* Get's the GCRule configured for the column family.
*/
public GCRule getGCRule() {
return rule;
}

/**
* Returns true if a GCRule has been configured for the family
* Returns true if a GCRule has been configured for the family.
*/
public boolean hasGCRule() {
return !RuleCase.RULE_NOT_SET.equals(rule.toProto().getRuleCase());
Expand Down
4 changes: 2 additions & 2 deletions ...cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/GCRules.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ public boolean equals(Object o) {
return false;
}
VersionRule that = (VersionRule) o;
return Objects.equal(builder, that.builder);
return Objects.equal(builder.build(), that.builder.build());
}

@Override
Expand Down Expand Up @@ -324,7 +324,7 @@ public boolean equals(Object o) {
return false;
}
DurationRule that = (DurationRule) o;
return Objects.equal(builder, that.builder);
return Objects.equal(builder.build(), that.builder.build());
}

@Override
Expand Down
89 changes: 32 additions & 57 deletions ...e-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/models/Table.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,86 +16,68 @@
package com.google.cloud.bigtable.admin.v2.models;

import com.google.api.core.InternalApi;
import com.google.bigtable.admin.v2.Table.ClusterState.ReplicationState;
import com.google.bigtable.admin.v2.TableName;
import com.google.common.base.MoreObjects;
import com.google.common.base.Objects;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import javax.annotation.Nonnull;

/** Wrapper for {@link Table} protocol buffer object */
public final class Table {
private final TableName tableName;
// TODO(igorbernstein2): avoid duplication of id as keys and embedded in the models
private final Map<String, ClusterState> clusterStates;
private final Map<String, ColumnFamily> columnFamilies;
private final String id;
private final String instanceId;
private final Map<String, ReplicationState> replicationStatesByClusterId;
private final List<ColumnFamily> columnFamilies;

@InternalApi
public static Table fromProto(@Nonnull com.google.bigtable.admin.v2.Table proto) {
ImmutableMap.Builder<String, ClusterState> clusterStates = ImmutableMap.builder();
ImmutableMap.Builder<String, ReplicationState> replicationStates = ImmutableMap.builder();

for (Entry<String, com.google.bigtable.admin.v2.Table.ClusterState> entry : proto.getClusterStatesMap().entrySet()) {
clusterStates.put(entry.getKey(), ClusterState.fromProto(entry.getKey(), entry.getValue()));
replicationStates.put(entry.getKey(), entry.getValue().getReplicationState());
}

ImmutableMap.Builder<String, ColumnFamily> columnFamilies = ImmutableMap.builder();
ImmutableList.Builder<ColumnFamily> columnFamilies = ImmutableList.builder();

for (Entry<String, com.google.bigtable.admin.v2.ColumnFamily> entry : proto.getColumnFamiliesMap().entrySet()) {
columnFamilies.put(entry.getKey(), ColumnFamily.fromProto(entry.getKey(), entry.getValue()));
columnFamilies.add(ColumnFamily.fromProto(entry.getKey(), entry.getValue()));
}

return new Table(
TableName.parse(proto.getName()),
clusterStates.build(),
replicationStates.build(),
columnFamilies.build());
}

private Table(TableName tableName,
Map<String, ClusterState> clusterStates,
Map<String, ColumnFamily> columnFamilies) {
this.tableName = tableName;
this.clusterStates = clusterStates;
Map<String, ReplicationState> replicationStatesByClusterId,
List<ColumnFamily> columnFamilies) {
this.instanceId = tableName.getInstance();
this.id = tableName.getTable();
this.replicationStatesByClusterId = replicationStatesByClusterId;
this.columnFamilies = columnFamilies;
}

/**
* Gets the unique name of the table in the format:
* projects/{project}/instances/{instance}/tables/{tableId}
*/
public TableName getTableName() {
return tableName;
/** Gets the table's id. */
public String getId() {
return id;
}

// TODO(igorbernstein2): don't expose the objects as both maps & lists
/**
* Returns state of the table by clusters in the instance as map of clusterId and {@link
* ClusterState}
*/
public Map<String, ClusterState> getClusterStatesMap() {
return clusterStates;
/** Gets the id of the instance that owns this Table. */
public String getInstanceId() {
return instanceId;
}

/**
* Returns a map of columfamilies in the table keyed by columnfamily and name
*/
public Map<String, ColumnFamily> getColumnFamiliesMap() {
return columnFamilies;
}

/**
* Returns state of the table by clusters in the instance as a Collection
*/
public Collection<ClusterState> getClusterStates() {
return clusterStates.values();
public Map<String, ReplicationState> getReplicationStatesByClusterId() {
return replicationStatesByClusterId;
}

/**
* Returns all columnfamilies in the table as a Collection
*/
public Collection<ColumnFamily> getColumnFamiles() {
return columnFamilies.values();
public List<ColumnFamily> getColumnFamilies() {
return columnFamilies;
}

@Override
Expand All @@ -107,22 +89,15 @@ public boolean equals(Object o) {
return false;
}
Table table = (Table) o;
return Objects.equal(tableName, table.tableName) &&
Objects.equal(clusterStates, table.clusterStates) &&
return Objects.equal(id, table.id) &&
Objects.equal(instanceId, table.instanceId) &&
Objects
.equal(replicationStatesByClusterId, table.replicationStatesByClusterId) &&
Objects.equal(columnFamilies, table.columnFamilies);
}

@Override
public int hashCode() {
return Objects.hashCode(tableName, clusterStates, columnFamilies);
}

@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("tableName", tableName)
.add("clusterStates", getClusterStates())
.add("columnFamiles", getColumnFamiles())
.toString();
return Objects.hashCode(id, instanceId, replicationStatesByClusterId, columnFamilies);
}
}
43 changes: 28 additions & 15 deletions ...admin/src/test/java/com/google/cloud/bigtable/admin/v2/it/BigtableTableAdminClientIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.bigtable.admin.v2.InstanceName;
import com.google.bigtable.admin.v2.TableName;
import com.google.cloud.bigtable.admin.v2.BigtableTableAdminClient;
import com.google.cloud.bigtable.admin.v2.models.ColumnFamily;
import com.google.cloud.bigtable.admin.v2.models.GCRules.DurationRule;
import com.google.cloud.bigtable.admin.v2.models.GCRules.IntersectionRule;
import com.google.cloud.bigtable.admin.v2.models.GCRules.UnionRule;
Expand All @@ -32,9 +33,11 @@
import com.google.cloud.bigtable.admin.v2.models.ModifyColumnFamiliesRequest;
import com.google.cloud.bigtable.admin.v2.models.ConsistencyToken;
import com.google.cloud.bigtable.admin.v2.models.Table;
import com.google.common.collect.Maps;
import com.google.protobuf.ByteString;
import java.io.IOException;
import java.util.List;
import java.util.Map;
import org.junit.AfterClass;
import org.junit.AssumptionViolatedException;
import org.junit.Before;
Expand Down Expand Up @@ -88,13 +91,18 @@ public void createTable() {
try {
Table tableResponse = tableAdmin.createTable(createTableReq);
assertNotNull(tableResponse);
assertEquals(tableId, tableResponse.getTableName().getTable());
assertEquals(2, tableResponse.getColumnFamiles().size());
assertFalse(tableResponse.getColumnFamiliesMap().get("cf1").hasGCRule());
assertTrue(tableResponse.getColumnFamiliesMap().get("cf2").hasGCRule());
assertEquals(tableId, tableResponse.getId());

Map<String, ColumnFamily> columnFamilyById = Maps.newHashMap();
for (ColumnFamily columnFamily : tableResponse.getColumnFamilies()) {
columnFamilyById.put(columnFamily.getId(), columnFamily);
}
assertEquals(2, tableResponse.getColumnFamilies().size());
assertFalse(columnFamilyById.get("cf1").hasGCRule());
assertTrue(columnFamilyById.get("cf2").hasGCRule());
assertEquals(
10,
((VersionRule) tableResponse.getColumnFamiliesMap().get("cf2").getGCRule())
((VersionRule) columnFamilyById.get("cf2").getGCRule())
.getMaxVersions());
} finally {
tableAdmin.deleteTable(tableId);
Expand Down Expand Up @@ -131,35 +139,40 @@ public void modifyFamilies() {
try {
tableAdmin.createTable(CreateTableRequest.of(tableId));
Table tableResponse = tableAdmin.modifyFamilies(modifyFamiliesReq);
assertEquals(5, tableResponse.getColumnFamiles().size());
assertNotNull(tableResponse.getColumnFamiliesMap().get("mf1"));
assertNotNull(tableResponse.getColumnFamiliesMap().get("mf2"));

Map<String, ColumnFamily> columnFamilyById = Maps.newHashMap();
for (ColumnFamily columnFamily : tableResponse.getColumnFamilies()) {
columnFamilyById.put(columnFamily.getId(), columnFamily);
}
assertEquals(5, columnFamilyById.size());
assertNotNull(columnFamilyById.get("mf1"));
assertNotNull(columnFamilyById.get("mf2"));
assertEquals(
2,
((UnionRule) tableResponse.getColumnFamiliesMap().get("mf1").getGCRule())
((UnionRule) columnFamilyById.get("mf1").getGCRule())
.getRulesList()
.size());
assertEquals(
1000,
((DurationRule) tableResponse.getColumnFamiliesMap().get("mf2").getGCRule())
((DurationRule) columnFamilyById.get("mf2").getGCRule())
.getMaxAge()
.getSeconds());
assertEquals(
20000,
((DurationRule) tableResponse.getColumnFamiliesMap().get("mf2").getGCRule())
((DurationRule) columnFamilyById.get("mf2").getGCRule())
.getMaxAge()
.getNano());
assertEquals(
2,
((IntersectionRule) tableResponse.getColumnFamiliesMap().get("mf3").getGCRule())
((IntersectionRule) columnFamilyById.get("mf3").getGCRule())
.getRulesList()
.size());
assertEquals(
360,
((DurationRule) tableResponse.getColumnFamiliesMap().get("mf4").getGCRule())
((DurationRule) columnFamilyById.get("mf4").getGCRule())
.getMaxAge()
.getSeconds());
assertNotNull(tableResponse.getColumnFamiliesMap().get("mf7"));
assertNotNull(columnFamilyById.get("mf7"));
} finally {
tableAdmin.deleteTable(tableId);
}
Expand All @@ -180,7 +193,7 @@ public void getTable() {
tableAdmin.createTable(CreateTableRequest.of(tableId));
Table tableResponse = tableAdmin.getTable(tableId);
assertNotNull(tableResponse);
assertEquals(tableId, tableResponse.getTableName().getTable());
assertEquals(tableId, tableResponse.getId());
} finally {
tableAdmin.deleteTable(tableId);
}
Expand Down
Loading

0 comments on commit 3cefb4f

Please sign in to comment.