-
Notifications
You must be signed in to change notification settings - Fork 40
Introduce StorageInfo #2756
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
Introduce StorageInfo #2756
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| package com.scalar.db.storage.cassandra; | ||
|
|
||
| import com.scalar.db.api.DistributedStorageMutationAtomicityUnitIntegrationTestBase; | ||
| import java.util.Collections; | ||
| import java.util.Map; | ||
| import java.util.Properties; | ||
|
|
||
| public class CassandraMutationAtomicityUnitIntegrationTest | ||
| extends DistributedStorageMutationAtomicityUnitIntegrationTestBase { | ||
|
|
||
| @Override | ||
| protected Properties getProperties(String testName) { | ||
| return CassandraEnv.getProperties(testName); | ||
| } | ||
|
|
||
| @Override | ||
| protected Map<String, String> getCreationOptions() { | ||
| return Collections.singletonMap(CassandraAdmin.REPLICATION_FACTOR, "1"); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| package com.scalar.db.storage.cosmos; | ||
|
|
||
| import com.scalar.db.api.DistributedStorageMutationAtomicityUnitIntegrationTestBase; | ||
| import java.util.Map; | ||
| import java.util.Properties; | ||
| import org.junit.jupiter.api.Disabled; | ||
|
|
||
| public class CosmosMutationAtomicityUnitIntegrationTest | ||
| extends DistributedStorageMutationAtomicityUnitIntegrationTestBase { | ||
|
|
||
| @Override | ||
| protected Properties getProperties(String testName) { | ||
| return CosmosEnv.getProperties(testName); | ||
| } | ||
|
|
||
| @Override | ||
| protected Map<String, String> getCreationOptions() { | ||
| return CosmosEnv.getCreationOptions(); | ||
| } | ||
|
|
||
| @Disabled("This test fails. It might be a bug") | ||
| @Override | ||
| public void | ||
| mutate_MutationsWithinRecordGiven_ShouldBehaveCorrectlyBaseOnMutationAtomicityUnit() {} | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| package com.scalar.db.storage.dynamo; | ||
|
|
||
| import com.scalar.db.api.DistributedStorageMutationAtomicityUnitIntegrationTestBase; | ||
| import java.util.Map; | ||
| import java.util.Properties; | ||
| import org.junit.jupiter.api.Disabled; | ||
|
|
||
| public class DynamoMutationAtomicityUnitIntegrationTest | ||
| extends DistributedStorageMutationAtomicityUnitIntegrationTestBase { | ||
|
|
||
| @Override | ||
| protected Properties getProperties(String testName) { | ||
| return DynamoEnv.getProperties(testName); | ||
| } | ||
|
|
||
| @Override | ||
| protected Map<String, String> getCreationOptions() { | ||
| return DynamoEnv.getCreationOptions(); | ||
| } | ||
|
|
||
| @Disabled("Transaction request cannot include multiple operations on one item in DynamoDB") | ||
| @Override | ||
| public void | ||
| mutate_MutationsWithinRecordGiven_ShouldBehaveCorrectlyBaseOnMutationAtomicityUnit() {} | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| package com.scalar.db.storage.jdbc; | ||
|
|
||
| import com.scalar.db.api.DistributedStorageMutationAtomicityUnitIntegrationTestBase; | ||
| import java.util.Properties; | ||
|
|
||
| public class JdbcDatabaseMutationAtomicityUnitIntegrationTest | ||
| extends DistributedStorageMutationAtomicityUnitIntegrationTestBase { | ||
|
|
||
| @Override | ||
| protected Properties getProperties(String testName) { | ||
| return JdbcEnv.getProperties(testName); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| package com.scalar.db.storage.multistorage; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| import com.scalar.db.api.DistributedStorageMutationAtomicityUnitIntegrationTestBase; | ||
| import com.scalar.db.api.Put; | ||
| import com.scalar.db.config.DatabaseConfig; | ||
| import com.scalar.db.exception.storage.ExecutionException; | ||
| import com.scalar.db.io.Key; | ||
| import java.util.Arrays; | ||
| import java.util.Properties; | ||
| import org.assertj.core.api.Assertions; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| public class MultiStorageMutationAtomicityUnitIntegrationTest | ||
| extends DistributedStorageMutationAtomicityUnitIntegrationTestBase { | ||
|
|
||
| @Override | ||
| public Properties getProperties(String testName) { | ||
| Properties properties = new Properties(); | ||
| properties.setProperty(DatabaseConfig.STORAGE, "multi-storage"); | ||
|
|
||
| // Define storages, jdbc and cassandra | ||
| properties.setProperty(MultiStorageConfig.STORAGES, "jdbc,cassandra"); | ||
|
|
||
| Properties propertiesForJdbc = MultiStorageEnv.getPropertiesForJdbc(testName); | ||
| for (String propertyName : propertiesForJdbc.stringPropertyNames()) { | ||
| properties.setProperty( | ||
| MultiStorageConfig.STORAGES | ||
| + ".jdbc." | ||
| + propertyName.substring(DatabaseConfig.PREFIX.length()), | ||
| propertiesForJdbc.getProperty(propertyName)); | ||
| } | ||
|
|
||
| Properties propertiesForCassandra = MultiStorageEnv.getPropertiesForCassandra(testName); | ||
| for (String propertyName : propertiesForCassandra.stringPropertyNames()) { | ||
| properties.setProperty( | ||
| MultiStorageConfig.STORAGES | ||
| + ".cassandra." | ||
| + propertyName.substring(DatabaseConfig.PREFIX.length()), | ||
| propertiesForCassandra.getProperty(propertyName)); | ||
| } | ||
|
|
||
| // Define namespace mappings | ||
| properties.setProperty( | ||
| MultiStorageConfig.NAMESPACE_MAPPING, | ||
| NAMESPACE1 + ":jdbc," + NAMESPACE2 + ":jdbc," + NAMESPACE3 + ":cassandra"); | ||
|
|
||
| // The default storage is jdbc | ||
| properties.setProperty(MultiStorageConfig.DEFAULT_STORAGE, "jdbc"); | ||
|
|
||
| // Add testName as a metadata schema suffix | ||
| properties.setProperty( | ||
| DatabaseConfig.SYSTEM_NAMESPACE_NAME, | ||
| DatabaseConfig.DEFAULT_SYSTEM_NAMESPACE_NAME + "_" + testName); | ||
|
|
||
| return properties; | ||
| } | ||
|
|
||
| @Test | ||
| public void mutate_MutationsAcrossStorageGiven_ShouldBehaveCorrectlyBaseOnMutationAtomicityUnit() | ||
| throws ExecutionException { | ||
| // Arrange | ||
| Put put1 = | ||
| Put.newBuilder() | ||
| .namespace(namespace1) | ||
| .table(TABLE1) | ||
| .partitionKey(Key.ofInt(COL_NAME1, 0)) | ||
| .clusteringKey(Key.ofInt(COL_NAME2, 1)) | ||
| .intValue(COL_NAME3, 1) | ||
| .build(); | ||
| Put put2 = | ||
| Put.newBuilder() | ||
| .namespace(namespace3) | ||
| .table(TABLE1) | ||
| .partitionKey(Key.ofInt(COL_NAME1, 1)) | ||
| .clusteringKey(Key.ofInt(COL_NAME2, 2)) | ||
| .intValue(COL_NAME3, 2) | ||
| .build(); | ||
|
|
||
| // Act | ||
| Exception exception = | ||
| Assertions.catchException(() -> storage.mutate(Arrays.asList(put1, put2))); | ||
|
|
||
| // Assert | ||
| assertThat(exception).isInstanceOf(IllegalArgumentException.class); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,62 @@ | ||||||
| package com.scalar.db.api; | ||||||
|
|
||||||
| public interface StorageInfo { | ||||||
| /** | ||||||
| * Returns the storage name. | ||||||
| * | ||||||
| * @return the storage name | ||||||
| */ | ||||||
| String getStorageName(); | ||||||
|
|
||||||
| /** | ||||||
| * Returns the mutation atomicity unit of the storage. | ||||||
| * | ||||||
| * @return the mutation atomicity unit of the storage | ||||||
| */ | ||||||
| MutationAtomicityUnit getMutationAtomicityUnit(); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
After some thought, I feel it's probably more proper to call it
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gemini says What do you think? 🙇
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also asked Gemini. It suggests Granularity.... I think it depends on how we ask. Question: We provide a transaction manager for various kinds of databases. The transaction manager pushes down write operations into the underlying databases based on the size that the databases can atomically write. To make each database provide the size information, I created a I have two candidate names for the method. Answer: Here’s a breakdown of why, along with some alternative ideas. Comparison of Your Candidates getMutationAtomicityGranularity(): Slightly better. 👍 getMutationAtomicityUnit(): Also very good. Other Ideas getAtomicityLevel() getAtomicWriteScope() Final Recommendation If you feel it's too much jargon, getAtomicityLevel() is a fantastic, simpler alternative that conveys the same meaning effectively.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you share the question as well? Anyway, Unit seems also correct, so I'm fine with Unit, though.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My question was as follows: In the context of transactions, which term is more appropriate to express the range in which updates can be performed atomically: MutationAtomicityUnit or MutationAtomicityGranularity? Below is an example using MutationAtomicityUnit: /**
* The mutation atomicity unit of the storage.
*
* <p>This enum defines the atomicity unit for mutations in the storage. It determines the scope
* of atomicity for mutations such as put and delete.
*/
enum MutationAtomicityUnit {
/**
* The atomicity unit is at the record level, meaning that mutations are performed atomically
* for each record.
*/
RECORD,
/**
* The atomicity unit is at the partition level, meaning that mutations are performed atomically
* for each partition.
*/
PARTITION,
/**
* The atomicity unit is at the table level, meaning that mutations are performed atomically for
* each table.
*/
TABLE,
/**
* The atomicity unit is at the namespace level, meaning that mutations are performed atomically
* for each namespace.
*/
NAMESPACE,
/**
* The atomicity unit is at the storage level, meaning that mutations are performed atomically
* for the entire storage.
*/
STORAGE
}
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on the discussion, since |
||||||
|
|
||||||
| /** | ||||||
| * Returns the maximum number of mutations that can be performed atomically in the storage. | ||||||
| * | ||||||
| * @return the maximum number of mutations that can be performed atomically in the storage | ||||||
| */ | ||||||
| int getMaxAtomicMutationsCount(); | ||||||
|
|
||||||
| /** | ||||||
| * The mutation atomicity unit of the storage. | ||||||
| * | ||||||
| * <p>This enum defines the atomicity unit for mutations in the storage. It determines the scope | ||||||
| * of atomicity for mutations such as put and delete. | ||||||
| */ | ||||||
| enum MutationAtomicityUnit { | ||||||
| /** | ||||||
| * The atomicity unit is at the record level, meaning that mutations are performed atomically | ||||||
| * for each record. | ||||||
| */ | ||||||
| RECORD, | ||||||
|
|
||||||
| /** | ||||||
| * The atomicity unit is at the partition level, meaning that mutations are performed atomically | ||||||
| * for each partition. | ||||||
| */ | ||||||
| PARTITION, | ||||||
|
|
||||||
| /** | ||||||
| * The atomicity unit is at the table level, meaning that mutations are performed atomically for | ||||||
| * each table. | ||||||
| */ | ||||||
| TABLE, | ||||||
|
|
||||||
| /** | ||||||
| * The atomicity unit is at the namespace level, meaning that mutations are performed atomically | ||||||
| * for each namespace. | ||||||
| */ | ||||||
| NAMESPACE, | ||||||
|
|
||||||
| /** | ||||||
| * The atomicity unit is at the storage level, meaning that mutations are performed atomically | ||||||
| * for the entire storage. | ||||||
| */ | ||||||
| STORAGE | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| package com.scalar.db.common; | ||
|
|
||
| import com.google.common.base.MoreObjects; | ||
| import com.scalar.db.api.StorageInfo; | ||
| import java.util.Objects; | ||
| import javax.annotation.concurrent.Immutable; | ||
|
|
||
| @Immutable | ||
| public class StorageInfoImpl implements StorageInfo { | ||
|
|
||
| private final String storageName; | ||
| private final MutationAtomicityUnit mutationAtomicityUnit; | ||
| private final int maxAtomicMutationsCount; | ||
|
|
||
| public StorageInfoImpl( | ||
| String storageName, | ||
| MutationAtomicityUnit mutationAtomicityUnit, | ||
| int maxAtomicMutationsCount) { | ||
| this.storageName = storageName; | ||
| this.mutationAtomicityUnit = mutationAtomicityUnit; | ||
| this.maxAtomicMutationsCount = maxAtomicMutationsCount; | ||
| } | ||
|
|
||
| @Override | ||
| public String getStorageName() { | ||
| return storageName; | ||
| } | ||
|
|
||
| @Override | ||
| public MutationAtomicityUnit getMutationAtomicityUnit() { | ||
| return mutationAtomicityUnit; | ||
| } | ||
|
|
||
| @Override | ||
| public int getMaxAtomicMutationsCount() { | ||
| return maxAtomicMutationsCount; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) { | ||
| return true; | ||
| } | ||
| if (!(o instanceof StorageInfoImpl)) { | ||
| return false; | ||
| } | ||
| StorageInfoImpl that = (StorageInfoImpl) o; | ||
| return getMaxAtomicMutationsCount() == that.getMaxAtomicMutationsCount() | ||
| && Objects.equals(getStorageName(), that.getStorageName()) | ||
| && getMutationAtomicityUnit() == that.getMutationAtomicityUnit(); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(getStorageName(), getMutationAtomicityUnit(), getMaxAtomicMutationsCount()); | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return MoreObjects.toStringHelper(this) | ||
| .add("storageName", storageName) | ||
| .add("mutationAtomicityUnit", mutationAtomicityUnit) | ||
| .add("maxAtomicMutationsCount", maxAtomicMutationsCount) | ||
| .toString(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can get the
StorageInfoinstance of the storage through this method.