diff --git a/.github/workflows/build-tag-publish.yml b/.github/workflows/build-tag-publish.yml index 2e38f27c..f21857c6 100644 --- a/.github/workflows/build-tag-publish.yml +++ b/.github/workflows/build-tag-publish.yml @@ -28,7 +28,7 @@ jobs: uses: gradle/wrapper-validation-action@v1 - name: Build with Gradle - run: ./gradlew clean build --debug + run: ./gradlew clean build - name: Start Docker Containers run: docker compose -f infra/recipes/docker-compose/oh-only/docker-compose.yml up -d --build @@ -52,7 +52,7 @@ jobs: - name: Bump version and push tag if: ${{ success() && github.ref == 'refs/heads/main' && github.repository == 'linkedin/openhouse' }} - uses: anothrNick/github-tag-action@1.67.0 + uses: anothrNick/github-tag-action@1.69.0 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} WITH_V: true # prefix for tag "v" diff --git a/SETUP.md b/SETUP.md index b34bb721..385ffcb8 100644 --- a/SETUP.md +++ b/SETUP.md @@ -15,7 +15,7 @@ OpenHouse locally on laptop. Script has been tested to work fine on MacOS. Multiple recipes are provided for locally bringing up a docker-compose environment that can be used for testing. -Config| docker-dompose Directory |Notes +Config| docker-compose Directory |Notes ---|--------------------------|--- Run OpenHouse Services Only | oh-only | Stores data on local filesystem within the application container, with in-memory database. Least resource consuming. Run OpenHouse Services on HDFS | oh-hadoop | Stores data on locally running Hadoop HDFS containers, with iceberg-backed database. diff --git a/apps/spark/build.gradle b/apps/spark/build.gradle index a2e4dcbd..123aa255 100644 --- a/apps/spark/build.gradle +++ b/apps/spark/build.gradle @@ -78,6 +78,7 @@ test.dependsOn ':integrations:spark:openhouse-spark-runtime_2.12:build' shadowJar { zip64 = true + archiveClassifier.set('uber') mergeServiceFiles() // merge META-INF/services configuration files to allow FileSystem to be discovered dependencies { // unnecessary dependencies from iceberg-spark3-runtime diff --git a/apps/spark/src/main/java/com/linkedin/openhouse/jobs/scheduler/JobsScheduler.java b/apps/spark/src/main/java/com/linkedin/openhouse/jobs/scheduler/JobsScheduler.java index c8879dea..aef86b0e 100644 --- a/apps/spark/src/main/java/com/linkedin/openhouse/jobs/scheduler/JobsScheduler.java +++ b/apps/spark/src/main/java/com/linkedin/openhouse/jobs/scheduler/JobsScheduler.java @@ -143,6 +143,7 @@ protected void run(JobConf.JobTypeEnum jobType, String taskType, boolean isDryRu taskFutures.add(executorService.submit(taskList.get(taskIndex))); } + int emptyStateJobCount = 0; for (int taskIndex = 0; taskIndex < taskList.size(); ++taskIndex) { Optional jobState = Optional.empty(); OperationTask task = taskList.get(taskIndex); @@ -171,16 +172,22 @@ protected void run(JobConf.JobTypeEnum jobType, String taskType, boolean isDryRu jobStateCountMap.put(JobState.CANCELLED, jobStateCountMap.get(JobState.CANCELLED) + 1); } } finally { - if (jobState.isPresent() && jobStateCountMap.containsKey(jobState.get())) { + if (jobState.isPresent()) { jobStateCountMap.put(jobState.get(), jobStateCountMap.get(jobState.get()) + 1); + } else { + emptyStateJobCount++; } } } log.info( - "Finishing scheduler, {} tasks completed successfully out of {} tasks, {} tasks cancelled due to timeout", - jobStateCountMap.get(JobState.SUCCEEDED), + "Finishing scheduler for job type {}, tasks stats: {} created, {} succeeded," + + " {} cancelled (timeout), {} failed, {} skipped (no state)", + jobType, taskList.size(), - jobStateCountMap.get(JobState.CANCELLED)); + jobStateCountMap.get(JobState.SUCCEEDED), + jobStateCountMap.get(JobState.CANCELLED), + jobStateCountMap.get(JobState.FAILED), + emptyStateJobCount); executorService.shutdown(); METER.counterBuilder("scheduler_end_count").build().add(1); reportSchedulerMetrics(jobStateCountMap, taskType, startTimeMillis); diff --git a/apps/spark/src/main/java/com/linkedin/openhouse/jobs/spark/Operations.java b/apps/spark/src/main/java/com/linkedin/openhouse/jobs/spark/Operations.java index 4359bac8..c731d911 100644 --- a/apps/spark/src/main/java/com/linkedin/openhouse/jobs/spark/Operations.java +++ b/apps/spark/src/main/java/com/linkedin/openhouse/jobs/spark/Operations.java @@ -3,11 +3,8 @@ import avro.shaded.com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Lists; import com.linkedin.openhouse.common.stats.model.IcebergTableStats; -import com.linkedin.openhouse.jobs.util.AppConstants; import com.linkedin.openhouse.jobs.util.SparkJobUtil; import com.linkedin.openhouse.jobs.util.TableStatsCollector; -import io.opentelemetry.api.common.AttributeKey; -import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.Meter; import java.io.IOException; import java.util.HashMap; @@ -15,11 +12,9 @@ import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.function.Predicate; -import java.util.stream.Collectors; import lombok.AccessLevel; import lombok.AllArgsConstructor; import lombok.extern.slf4j.Slf4j; -import org.apache.commons.lang.StringUtils; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.LocatedFileStatus; @@ -36,8 +31,6 @@ import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.spark.actions.SparkActions; -import org.apache.spark.sql.Dataset; -import org.apache.spark.sql.Row; import org.apache.spark.sql.SparkSession; import scala.collection.JavaConverters; @@ -240,51 +233,10 @@ public ExpireSnapshots.Result expireSnapshots(Table table, long expireBeforeTime */ public void runRetention( String fqtn, String columnName, String columnPattern, String granularity, int count) { - checkRecords(fqtn, columnName, columnPattern); final String statement = SparkJobUtil.createDeleteStatement(fqtn, columnName, columnPattern, granularity, count); - Dataset dsWithExpiredRows = - spark.sql( - SparkJobUtil.createSelectLimitStatement( - fqtn, columnName, columnPattern, granularity, count, 1)); - if (!dsWithExpiredRows.isEmpty()) { - spark.sql(statement).show(); - } - } - - /** - * CheckRecords verifies a sample from fqtn against @columnPattern to validate parsing with - * columnPattern. The metrics emitted helps to verify if records in table are not adhering to - * DateTimeFormat standards - * - * @param fqtn - fully-qualified table name - * @param columnName - retention column name - * @param columnPattern - retention column pattern - */ - public void checkRecords(String fqtn, String columnName, String columnPattern) { - String quotedFqtn = SparkJobUtil.getQuotedFqtn(fqtn); - if (!StringUtils.isBlank(columnPattern)) { - String query = - String.format( - "SELECT COALESCE(to_timestamp(%s, '%s'), 'Invalid') AS parsed_timestamp FROM %s LIMIT 10", - columnName, columnPattern, quotedFqtn); - log.info("Pattern verification query {}", query); - List resultDF = spark.sql(query).collectAsList(); - if (resultDF.stream() - .map(r -> r.getString(r.fieldIndex("parsed_timestamp"))) - .collect(Collectors.toList()) - .contains("Invalid")) { - meter - .counterBuilder(AppConstants.INCOMPATIBLE_DATE_COLUMN) - .build() - .add(1, Attributes.of(AttributeKey.stringKey(AppConstants.TABLE_NAME), fqtn)); - log.warn( - "Failed to parse column {} with provided retention column pattern {} for table {}", - columnName, - columnPattern, - fqtn); - } - } + log.info("deleting records from table: {}", fqtn); + spark.sql(statement); } private Path getTrashPath(String path, String filePath, String trashDir) { diff --git a/apps/spark/src/main/java/com/linkedin/openhouse/jobs/util/SparkJobUtil.java b/apps/spark/src/main/java/com/linkedin/openhouse/jobs/util/SparkJobUtil.java index 195797d5..f0505bf1 100644 --- a/apps/spark/src/main/java/com/linkedin/openhouse/jobs/util/SparkJobUtil.java +++ b/apps/spark/src/main/java/com/linkedin/openhouse/jobs/util/SparkJobUtil.java @@ -47,7 +47,7 @@ private SparkJobUtil() {} result: records will be filtered from deletion */ private static final String RETENTION_CONDITION_WITH_PATTERN_TEMPLATE = - "to_date(substring(%s, 0, CHAR_LENGTH('%s')), '%s') < date_trunc('%s', current_timestamp() - INTERVAL %s %ss)"; + "%s < cast(date_format(current_timestamp() - INTERVAL %s %ss, '%s') as string)"; public static String createDeleteStatement( String fqtn, String columnName, String columnPattern, String granularity, int count) { @@ -59,11 +59,9 @@ public static String createDeleteStatement( String.format( RETENTION_CONDITION_WITH_PATTERN_TEMPLATE, columnName, - columnPattern, - columnPattern, - granularity, count, - granularity)); + granularity, + columnPattern)); log.info( "Table: {}. Column pattern: {}, columnName {}, granularity {}s, " + "retention query: {}", fqtn, @@ -89,47 +87,6 @@ public static String createDeleteStatement( } } - public static String createSelectLimitStatement( - String fqtn, - String columnName, - String columnPattern, - String granularity, - int count, - int limit) { - if (!StringUtils.isBlank(columnPattern)) { - String query = - String.format( - "SELECT * FROM %s WHERE %s limit %d", - getQuotedFqtn(fqtn), - String.format( - RETENTION_CONDITION_WITH_PATTERN_TEMPLATE, - columnName, - columnPattern, - columnPattern, - granularity, - count, - granularity), - limit); - log.info("Table: {} column pattern provided: {} selectQuery: {}", fqtn, columnPattern, query); - return query; - } else { - String query = - String.format( - "SELECT * FROM %s WHERE %s limit %d", - getQuotedFqtn(fqtn), - String.format( - RETENTION_CONDITION_TEMPLATE, - granularity, - columnName, - granularity, - count, - granularity), - limit); - log.info("Table: {} No column pattern provided: selectQuery: {}", fqtn, query); - return query; - } - } - public static String getQuotedFqtn(String fqtn) { String[] fqtnTokens = fqtn.split("\\."); // adding single quotes around fqtn for cases when db and/or tableName has special character(s), diff --git a/apps/spark/src/main/java/com/linkedin/openhouse/jobs/util/TableStatsCollectorUtil.java b/apps/spark/src/main/java/com/linkedin/openhouse/jobs/util/TableStatsCollectorUtil.java index b39fd970..54974f25 100644 --- a/apps/spark/src/main/java/com/linkedin/openhouse/jobs/util/TableStatsCollectorUtil.java +++ b/apps/spark/src/main/java/com/linkedin/openhouse/jobs/util/TableStatsCollectorUtil.java @@ -4,6 +4,7 @@ import com.linkedin.openhouse.common.stats.model.IcebergTableStats; import java.io.IOException; +import java.util.Optional; import java.util.stream.StreamSupport; import lombok.extern.slf4j.Slf4j; import org.apache.hadoop.fs.FileSystem; @@ -82,20 +83,29 @@ protected static IcebergTableStats populateStatsOfAllReferencedFiles( */ protected static IcebergTableStats populateStatsForSnapshots( String fqtn, Table table, SparkSession spark, IcebergTableStats stats) { - long snapshotId = table.currentSnapshot().snapshotId(); - Dataset allDataFiles = getAllDataFilesCount(table, spark, MetadataTableType.FILES); - long countOfDataFiles = allDataFiles.count(); + Dataset dataFiles = getAllDataFilesCount(table, spark, MetadataTableType.FILES); + long countOfDataFiles = dataFiles.count(); // Calculate sum of file sizes in bytes in above allDataFiles - long sumOfFileSizeBytes = getSumOfFileSizeBytes(allDataFiles); + long sumOfFileSizeBytes = getSumOfFileSizeBytes(dataFiles); + + Long currentSnapshotId = + Optional.ofNullable(table.currentSnapshot()) + .map(snapshot -> snapshot.snapshotId()) + .orElse(null); + + Long currentSnapshotTimestamp = + Optional.ofNullable(table.currentSnapshot()) + .map(snapshot -> snapshot.timestampMillis()) + .orElse(null); log.info( "Table: {}, Count of total Data files: {}, Sum of file sizes in bytes: {} for snaphot: {}", fqtn, countOfDataFiles, sumOfFileSizeBytes, - snapshotId); + currentSnapshotId); // Find minimum timestamp of all snapshots where snapshots is iterator Long oldestSnapshotTimestamp = @@ -106,8 +116,8 @@ protected static IcebergTableStats populateStatsForSnapshots( return stats .toBuilder() - .currentSnapshotId(snapshotId) - .currentSnapshotTimestamp(table.currentSnapshot().timestampMillis()) + .currentSnapshotId(currentSnapshotId) + .currentSnapshotTimestamp(currentSnapshotTimestamp) .oldestSnapshotTimestamp(oldestSnapshotTimestamp) .numCurrentSnapshotReferencedDataFiles(countOfDataFiles) .totalCurrentSnapshotReferencedDataFilesSizeInBytes(sumOfFileSizeBytes) @@ -168,11 +178,11 @@ protected static IcebergTableStats populateTableMetadata(Table table, IcebergTab .tableType(table.properties().get(getCanonicalFieldName("tableType"))) .tableCreator((table.properties().get(getCanonicalFieldName("tableCreator")))) .tableCreationTimestamp( - table.properties().containsKey("creationTime") + table.properties().containsKey(getCanonicalFieldName("creationTime")) ? Long.parseLong(table.properties().get(getCanonicalFieldName("creationTime"))) : 0) .tableLastUpdatedTimestamp( - table.properties().containsKey("lastModifiedTime") + table.properties().containsKey(getCanonicalFieldName("lastModifiedTime")) ? Long.parseLong(table.properties().get(getCanonicalFieldName("lastModifiedTime"))) : 0) .tableUUID(table.properties().get(getCanonicalFieldName("tableUUID"))) @@ -217,6 +227,8 @@ private static Dataset getAllDataFilesCount( } private static long getSumOfFileSizeBytes(Dataset allDataFiles) { + if (allDataFiles.isEmpty()) return 0; + return allDataFiles .agg(org.apache.spark.sql.functions.sum("file_size_in_bytes")) .first() diff --git a/apps/spark/src/test/java/com/linkedin/openhouse/jobs/spark/OperationsTest.java b/apps/spark/src/test/java/com/linkedin/openhouse/jobs/spark/OperationsTest.java index 1db4cbef..f66a3046 100644 --- a/apps/spark/src/test/java/com/linkedin/openhouse/jobs/spark/OperationsTest.java +++ b/apps/spark/src/test/java/com/linkedin/openhouse/jobs/spark/OperationsTest.java @@ -55,6 +55,7 @@ public void testRetentionSparkAppWithStringPartitionColumns() throws Exception { final String tableName3 = "db.test_retention_string_partition3"; final String tableName4 = "db.test_retention_string_partition4"; final String tableName5 = "db.test_retention_string_partition5"; + final String tableName6 = "db.test_retention_string_partition6"; List rowValue = new ArrayList<>(); try (Operations ops = Operations.withCatalog(getSparkSession(), meter)) { @@ -75,7 +76,7 @@ public void testRetentionSparkAppWithStringPartitionColumns() throws Exception { rowValue.add("202%s-07-2218:46:19-0700"); runRetentionJobWithStringPartitionColumns( ops, tableName3, rowValue, "datePartition", "yyyy-MM-ddHH:mm:ssZ", "day"); - verifyRowCount(ops, tableName3, 3); + verifyRowCount(ops, tableName3, 0); rowValue.clear(); rowValue.add("202%s-07-16-12"); @@ -89,9 +90,9 @@ public void testRetentionSparkAppWithStringPartitionColumns() throws Exception { rowValue.clear(); rowValue.add("202%s-07-16-12"); - // Rows with format different than the pattern provided, parsing fails silently for such - // values and date - // will not be deleted + // Rows with format different than the pattern provided. These rows will be deleted even + // though formats are + // different due to string comparison logic rowValue.add("202%s-07-2218:46:19-0700"); // Rows with current date which are not to be deleted List currentDates = @@ -102,13 +103,12 @@ public void testRetentionSparkAppWithStringPartitionColumns() throws Exception { rowValue.add(dateToday); runRetentionJobWithStringPartitionColumns( ops, tableName4, rowValue, "datePartition", "yyyy-MM-dd-HH", "day"); - verifyRowCount(ops, tableName4, 6); + verifyRowCount(ops, tableName4, 3); rowValue.clear(); - // Test case to show that difference in data format and columnPattern format can lead to - // data not being deleted and put table out of compliance. - // Data format and pattern are different in terms of delimiter which makes is inconsistent. - // to_date cast fails silently. + // Test case to show that difference in data format and columnPattern format is not blocking + // delete ops. + // Data format and pattern are different in terms of delimiter which makes them inconsistent. List currentDatesFormatMismatched = ops.spark() .sql( @@ -120,8 +120,17 @@ public void testRetentionSparkAppWithStringPartitionColumns() throws Exception { ops.spark() .sql("select * from openhouse.db.test_retention_string_partition5") .collectAsList(); - verifyRowCount(ops, tableName5, 3); + verifyRowCount(ops, tableName5, 0); rowValue.clear(); + + // Test to validate the latest snapshot added by retention delete ops is of type `delete` + rowValue.add("202%s-07-16-12"); + runRetentionJobWithStringPartitionColumns( + ops, tableName6, rowValue, "datePartition", "yyyy-MM-dd-HH", "day"); + verifyRowCount(ops, tableName6, 0); + rowValue.clear(); + List operations = getSnapshotOperationTypes(ops, tableName6); + Assertions.assertEquals(operations.get(0), "delete"); } } @@ -138,7 +147,7 @@ private void runRetentionJobWithStringPartitionColumns( } @Test - public void testRetentionDoesNotCreateSnapshotsOnNoOpDelete() throws Exception { + public void testRetentionCreatesSnapshotsOnNoOpDelete() throws Exception { final String tableName = "db_test.test_retention_sql"; try (Operations ops = Operations.withCatalog(getSparkSession(), meter)) { prepareTable(ops, tableName); @@ -149,7 +158,7 @@ public void testRetentionDoesNotCreateSnapshotsOnNoOpDelete() throws Exception { ops.runRetention(tableName, "ts", "", "day", 2); verifyRowCount(ops, tableName, 4); List snapshotsAfter = getSnapshotIds(ops, tableName); - Assertions.assertEquals(snapshots.size(), snapshotsAfter.size()); + Assertions.assertEquals(snapshots.size() + 1, snapshotsAfter.size()); } } @@ -524,12 +533,23 @@ public void testCollectTableStats() throws Exception { final int numInserts = 3; try (Operations ops = Operations.withCatalog(getSparkSession(), meter)) { prepareTable(ops, tableName); - populateTable(ops, tableName, 1); IcebergTableStats stats = ops.collectTableStats(tableName); + + // Validate empty data files case + Assertions.assertEquals(stats.getNumReferencedDataFiles(), 0); + Assertions.assertEquals(stats.getNumExistingMetadataJsonFiles(), 1); + long modifiedTimeStamp = System.currentTimeMillis(); + + populateTable(ops, tableName, 1); + stats = ops.collectTableStats(tableName); Assertions.assertEquals(stats.getNumReferencedDataFiles(), 1); + Assertions.assertTrue(stats.getTableLastUpdatedTimestamp() >= modifiedTimeStamp); + + // Capture first snapshot timestamp Table table = ops.getTable(tableName); long oldestSnapshot = table.currentSnapshot().timestampMillis(); + // Add more records and validate other stats populateTable(ops, tableName, numInserts); table = ops.getTable(tableName); log.info("Loaded table {}, location {}", table.name(), table.location()); @@ -567,7 +587,7 @@ private void verifyPolicies( Assertions.assertEquals(policies.getSharingEnabled().booleanValue(), expectedSharing); } - private void verifyRowCount(Operations ops, String tableName, int expectedRowCount) { + private static void verifyRowCount(Operations ops, String tableName, int expectedRowCount) { List resultRows = ops.spark().sql(String.format("SELECT * FROM %s", tableName)).collectAsList(); Assertions.assertEquals(expectedRowCount, resultRows.size()); @@ -636,7 +656,10 @@ private static void prepareTable(Operations ops, String tableName, boolean isPar private static void prepareTableWithStringColumn(Operations ops, String tableName) { ops.spark().sql(String.format("DROP TABLE IF EXISTS %s", tableName)).show(); ops.spark() - .sql(String.format("CREATE TABLE %s (data string, datePartition String)", tableName)) + .sql( + String.format( + "CREATE TABLE %s (data string, datePartition String) PARTITIONED by (datePartition)", + tableName)) .show(); ops.spark().sql(String.format("DESCRIBE %s", tableName)).show(); } @@ -672,7 +695,7 @@ private static void checkSnapshots(Table table, List expectedSnapshotIds) } private static List getSnapshotIds(Operations ops, String tableName) { - log.info("Getting snapshots"); + log.info("Getting snapshot Ids"); List snapshots = ops.spark().sql(String.format("SELECT * FROM %s.snapshots", tableName)).collectAsList(); snapshots.forEach(s -> log.info(s.toString())); @@ -681,6 +704,18 @@ private static List getSnapshotIds(Operations ops, String tableName) { .collect(Collectors.toList()); } + private static List getSnapshotOperationTypes(Operations ops, String tableName) { + log.info("Getting snapshot Operations"); + List ordered_snapshots = + ops.spark() + .sql(String.format("SELECT * FROM %s.snapshots order by committed_at desc", tableName)) + .collectAsList(); + ordered_snapshots.forEach(s -> log.info(s.toString())); + return ordered_snapshots.stream() + .map(r -> r.getString(r.fieldIndex("operation"))) + .collect(Collectors.toList()); + } + private static List> getDataFiles(Operations ops, String tableName) { List dataFiles = ops.spark().sql(String.format("SELECT * FROM %s.data_files", tableName)).collectAsList(); diff --git a/apps/spark/src/test/java/com/linkedin/openhouse/jobs/util/SparkJobUtilTest.java b/apps/spark/src/test/java/com/linkedin/openhouse/jobs/util/SparkJobUtilTest.java index 6769300e..33640800 100644 --- a/apps/spark/src/test/java/com/linkedin/openhouse/jobs/util/SparkJobUtilTest.java +++ b/apps/spark/src/test/java/com/linkedin/openhouse/jobs/util/SparkJobUtilTest.java @@ -20,32 +20,13 @@ void testGetQuotedFqtn() { "`db-dashed`.`table-name`", SparkJobUtil.getQuotedFqtn("db-dashed.table-name")); } - @Test - void testCreateSelectLimitStatement() { - String statement = - SparkJobUtil.createSelectLimitStatement("table.name", "attribute", "", "day", 3, 1); - Assertions.assertEquals( - statement, - "SELECT * FROM `table`.`name` WHERE date_trunc('day', attribute) < date_trunc('day', current_timestamp() - INTERVAL 3 days) limit 1"); - } - - @Test - void testCreateSelectLimitStatementWithStringColumnPartition() { - String expected = - "SELECT * FROM `table`.`name` WHERE to_date(substring(attribute, 0, CHAR_LENGTH('yyyy-MM-dd')), 'yyyy-MM-dd') < date_trunc('day', current_timestamp() - INTERVAL 3 days) limit 1"; - String actual = - SparkJobUtil.createSelectLimitStatement( - "table.name", "attribute", "yyyy-MM-dd", "day", 3, 1); - Assertions.assertEquals(expected, actual); - } - @Test void testCreateDeleteStatementWithStringColumnPartition() { String expected = - "DELETE FROM `db`.`table-name` WHERE to_date(substring(string_partition, 0, CHAR_LENGTH('yyyy-MM-dd-HH')), 'yyyy-MM-dd-HH') < date_trunc('day', current_timestamp() - INTERVAL 2 days)"; + "DELETE FROM `db`.`table-name` WHERE string_partition < cast(date_format(current_timestamp() - INTERVAL 2 DAYs, 'yyyy-MM-dd-HH') as string)"; Assertions.assertEquals( expected, SparkJobUtil.createDeleteStatement( - "db.table-name", "string_partition", "yyyy-MM-dd-HH", "day", 2)); + "db.table-name", "string_partition", "yyyy-MM-dd-HH", "DAY", 2)); } } diff --git a/cluster/configs/src/main/java/com/linkedin/openhouse/cluster/configs/ClusterProperties.java b/cluster/configs/src/main/java/com/linkedin/openhouse/cluster/configs/ClusterProperties.java index e722382f..5678dbf2 100644 --- a/cluster/configs/src/main/java/com/linkedin/openhouse/cluster/configs/ClusterProperties.java +++ b/cluster/configs/src/main/java/com/linkedin/openhouse/cluster/configs/ClusterProperties.java @@ -54,7 +54,7 @@ public class ClusterProperties { @Value("${cluster.housetables.database.type:IN_MEMORY}") private String clusterHouseTablesDatabaseType; - @Value("${cluster.housetables.database.url:jdbc:h2:mem:htsdb;DB_CLOSE_DELAY=-1}") + @Value("${cluster.housetables.database.url:jdbc:h2:mem:htsdb;MODE=MYSQL;DB_CLOSE_DELAY=-1}") private String clusterHouseTablesDatabaseUrl; @Value("${HTS_DB_USER:}") diff --git a/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/BaseStorage.java b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/BaseStorage.java new file mode 100644 index 00000000..2c992bfe --- /dev/null +++ b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/BaseStorage.java @@ -0,0 +1,44 @@ +package com.linkedin.openhouse.cluster.storage; + +import com.linkedin.openhouse.cluster.storage.configs.StorageProperties; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import org.springframework.beans.factory.annotation.Autowired; + +/** + * The BaseStorage class is an abstract class that implements the Storage interface. It provides + * common functionality for all storage implementations. + */ +public abstract class BaseStorage implements Storage { + + @Autowired private StorageProperties storageProperties; + + /** + * Check if the storage is configured. + * + *

The storage is considered configured if type is defined in the storage properties. + * + * @return true if the storage is configured, false otherwise + */ + @Override + public boolean isConfigured() { + return Optional.ofNullable(storageProperties.getTypes()) + .map(types -> types.containsKey(getType().getValue())) + .orElse(false); + } + + /** + * Get the properties of the storage. + * + * @return a copy of map of properties of the storage + */ + @Override + public Map getProperties() { + return Optional.ofNullable(storageProperties.getTypes()) + .map(types -> types.get(getType().getValue())) + .map(StorageProperties.StorageTypeProperties::getParameters) + .map(HashMap::new) + .orElseGet(HashMap::new); + } +} diff --git a/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/Storage.java b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/Storage.java new file mode 100644 index 00000000..b0a8f079 --- /dev/null +++ b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/Storage.java @@ -0,0 +1,51 @@ +package com.linkedin.openhouse.cluster.storage; + +import java.util.Map; + +/** + * The Storage interface represents a storage system in OpenHouse. It provides methods to check if + * the storage is configured, retrieve properties of the storage, get the type of the storage, and + * get a client to interact with the storage. + * + *

Implementations of this interface should provide the specific logic for each type of storage. + * For example, the {@link com.linkedin.openhouse.cluster.storage.local.LocalStorage} class is an + * implementation of this interface for local storage, and it uses a {@link + * com.linkedin.openhouse.cluster.storage.local.LocalStorageClient} to interact with the local file + * system. + */ +public interface Storage { + + /** + * Check if the storage is configured. + * + *

The storage is considered configured if {@link + * com.linkedin.openhouse.cluster.storage.configs.StorageProperties} has type defined for it + * + * @return true if the storage is configured, false otherwise + */ + boolean isConfigured(); + + /** + * Get the properties of the storage. + * + * @return a map of properties of the storage + */ + Map getProperties(); + + /** + * Get the type of the storage. + * + *

Please refer to {@link StorageType} for the list of supported storage types. An example type + * of the local storage that can be returned {@link StorageType.Type#LOCAL}. + * + * @return the type of the storage + */ + StorageType.Type getType(); + + /** + * Get a client to interact with the storage. + * + * @return a client to interact with the storage + */ + StorageClient getClient(); +} diff --git a/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/StorageClient.java b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/StorageClient.java new file mode 100644 index 00000000..524bc404 --- /dev/null +++ b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/StorageClient.java @@ -0,0 +1,43 @@ +package com.linkedin.openhouse.cluster.storage; + +/** + * The StorageClient interface represents a client to interact with a storage system in OpenHouse. + * It provides a method to get the native client of the storage system. + * + *

Implementations of this interface should provide the specific logic for each type of storage + * client. For example, the {@link com.linkedin.openhouse.cluster.storage.local.LocalStorageClient} + * class is an implementation of this interface for local storage, and it uses an Apache Hadoop + * {@link org.apache.hadoop.fs.FileSystem} to interact with the local file system. + * + * @param the type of the native client of the storage system + */ +public interface StorageClient { + + /** + * Get the native client of the storage system. + * + * @return the native client of the storage system + */ + T getNativeClient(); + + /** + * Get the endpoint of the storage system. + * + *

Example: For HDFS, the endpoint could be "hdfs://localhost:9000". For local file system, the + * endpoint could be "file://". For S3, the endpoint could be "s3://". + * + * @return the endpoint of the storage system + */ + String getEndpoint(); + + /** + * Get the root prefix for OpenHouse on the storage system. + * + *

Root prefix should include the bucket-name plus any additional path components. Example: For + * HDFS, the root prefix could be "/data/openhouse". For local file system, the root prefix could + * be "/tmp". For S3, the root prefix could be "/bucket-name/key/prefix/to/openhouse". + * + * @return the root prefix for OpenHouse on the storage system + */ + String getRootPrefix(); +} diff --git a/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/StorageManager.java b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/StorageManager.java new file mode 100644 index 00000000..4fa40d09 --- /dev/null +++ b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/StorageManager.java @@ -0,0 +1,96 @@ +package com.linkedin.openhouse.cluster.storage; + +import static com.linkedin.openhouse.cluster.storage.StorageType.LOCAL; + +import com.google.common.base.Preconditions; +import com.linkedin.openhouse.cluster.storage.configs.StorageProperties; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import javax.annotation.PostConstruct; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; +import org.springframework.util.CollectionUtils; +import org.springframework.util.StringUtils; + +/** + * The StorageManager class is responsible for managing the storage types and providing the + * appropriate storage implementation based on the configuration. + */ +@Component +public class StorageManager { + + @Autowired StorageProperties storageProperties; + + @Autowired StorageType storageType; + + @Autowired List storages; + + /** + * Validate the storage properties. + * + *

It validates storage properties as follows: + * + *

1. If any storage type is configured, then default type must be set. Alternatively, if a + * default type is not set, then storage types should also be null or empty. **valid** + * + *

2. If default-type is set, then the value of the default type must exist in the configured + * storage types. **valid** + * + *

all other configurations are **invalid** + */ + @PostConstruct + public void validateProperties() { + String clusterYamlError = "Cluster yaml is incorrectly configured: "; + if (StringUtils.hasText(storageProperties.getDefaultType())) { + // default-type is configured, types should contain the default-type + Preconditions.checkArgument( + !CollectionUtils.isEmpty(storageProperties.getTypes()) + && storageProperties.getTypes().containsKey(storageProperties.getDefaultType()), + clusterYamlError + + "storage types should contain the default-type: " + + storageProperties.getDefaultType()); + } else { + // default-type is not configured, types should be null or empty + Preconditions.checkArgument( + CollectionUtils.isEmpty(storageProperties.getTypes()), + clusterYamlError + "default-type must be set if storage types are configured"); + } + try { + Optional.ofNullable(storageProperties.getDefaultType()).ifPresent(storageType::fromString); + Optional.ofNullable(storageProperties.getTypes()) + .map(Map::keySet) + .ifPresent(keyset -> keyset.forEach(key -> storageType.fromString(key))); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException(clusterYamlError + e.getMessage()); + } + } + + /** + * Get the default storage. + * + * @return the default storage + */ + public Storage getDefaultStorage() { + if (!StringUtils.hasText(storageProperties.getDefaultType())) { + return getStorage(LOCAL); + } + return getStorage(storageType.fromString(storageProperties.getDefaultType())); + } + + /** + * Get the storage based on the storage type. + * + * @param storageType the storage type + * @return the storage + */ + public Storage getStorage(StorageType.Type storageType) { + for (Storage storage : storages) { + if (storage.getType().equals(storageType) && storage.isConfigured()) { + return storage; + } + } + throw new IllegalArgumentException( + "No configured storage found for type: " + storageType.getValue()); + } +} diff --git a/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/StorageType.java b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/StorageType.java new file mode 100644 index 00000000..a1cf580a --- /dev/null +++ b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/StorageType.java @@ -0,0 +1,37 @@ +package com.linkedin.openhouse.cluster.storage; + +import lombok.AllArgsConstructor; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.ToString; +import org.springframework.stereotype.Component; + +/** + * Enum for supported storage types. + * + *

New types should be added here as public static final fields, and their corresponding + * implementations should be added to the fromString method. + */ +@Component +public class StorageType { + public static final Type HDFS = new Type("hdfs"); + public static final Type LOCAL = new Type("local"); + + @AllArgsConstructor + @EqualsAndHashCode + @ToString(includeFieldNames = false) + @Getter + public static class Type { + private String value; + } + + public Type fromString(String type) { + if (HDFS.getValue().equals(type)) { + return HDFS; + } else if (LOCAL.getValue().equals(type)) { + return LOCAL; + } else { + throw new IllegalArgumentException("Unknown storage type: " + type); + } + } +} diff --git a/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/configs/StorageProperties.java b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/configs/StorageProperties.java new file mode 100644 index 00000000..b363ee0d --- /dev/null +++ b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/configs/StorageProperties.java @@ -0,0 +1,44 @@ +package com.linkedin.openhouse.cluster.storage.configs; + +import com.linkedin.openhouse.cluster.configs.YamlPropertySourceFactory; +import java.util.HashMap; +import java.util.Map; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.PropertySource; + +/** + * This class represents the storage properties for the cluster. It includes the default storage + * type and a map of different storage types. Each storage type has its own properties such as root + * path, endpoint, and parameters. For list of supported storage types, see {@link + * com.linkedin.openhouse.cluster.storage.StorageType}. + */ +@Configuration +@ConfigurationProperties(prefix = "cluster.storages") +@PropertySource( + name = "clusterStorage", + value = "file:${OPENHOUSE_CLUSTER_CONFIG_PATH:/var/config/cluster.yaml}", + factory = YamlPropertySourceFactory.class, + ignoreResourceNotFound = true) +@Getter +@Setter +public class StorageProperties { + private String defaultType; + private Map types; + + @Getter + @Setter + @AllArgsConstructor + @NoArgsConstructor + @Builder(toBuilder = true) + public static class StorageTypeProperties { + private String rootPath; + private String endpoint; + @Builder.Default private Map parameters = new HashMap<>(); + } +} diff --git a/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/filesystem/DelegationTokenRefreshSchedulingConfig.java b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/filesystem/DelegationTokenRefreshSchedulingConfig.java deleted file mode 100644 index cf9c2d5d..00000000 --- a/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/filesystem/DelegationTokenRefreshSchedulingConfig.java +++ /dev/null @@ -1,42 +0,0 @@ -package com.linkedin.openhouse.cluster.storage.filesystem; - -import com.linkedin.openhouse.cluster.configs.YamlPropertySourceFactory; -import lombok.extern.slf4j.Slf4j; -import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.PropertySource; -import org.springframework.scheduling.annotation.EnableScheduling; - -/** - * Configuration class to conditionally enable/disable creation of {@link - * com.linkedin.openhouse.cluster.storage.filesystem.DelegationTokenRefresher} bean and enable - * delegation token refresh scheduling. - */ -@Configuration -@EnableScheduling -@Slf4j -@PropertySource( - name = "cluster", - value = "file:${OPENHOUSE_CLUSTER_CONFIG_PATH:/var/config/cluster.yaml}", - factory = YamlPropertySourceFactory.class, - ignoreResourceNotFound = true) -public class DelegationTokenRefreshSchedulingConfig { - - /** - * ConditionalOnProperty annotation works directly on the property defined in the config file i.e. - * cluster.yaml and {@link com.linkedin.openhouse.cluster.configs.ClusterProperties} can't be used - * here directly. Create DelegationTokenRefresher bean if the property value is defined as true, - * otherwise don't create. - * - * @return DelegationTokenRefresher - */ - @Bean - @ConditionalOnProperty( - value = "cluster.storage.hadoop.token.refresh.enabled", - havingValue = "true") - public DelegationTokenRefresher getDelegationTokenRefresher() { - log.info("Creating DelegationTokenRefresher bean....."); - return new DelegationTokenRefresher(); - } -} diff --git a/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/filesystem/DelegationTokenRefresher.java b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/hdfs/HdfsDelegationTokenRefresher.java similarity index 59% rename from cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/filesystem/DelegationTokenRefresher.java rename to cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/hdfs/HdfsDelegationTokenRefresher.java index 4daf3e03..75802394 100644 --- a/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/filesystem/DelegationTokenRefresher.java +++ b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/hdfs/HdfsDelegationTokenRefresher.java @@ -1,4 +1,4 @@ -package com.linkedin.openhouse.cluster.storage.filesystem; +package com.linkedin.openhouse.cluster.storage.hdfs; import static org.apache.hadoop.security.UserGroupInformation.HADOOP_TOKEN_FILE_LOCATION; @@ -7,6 +7,7 @@ import java.io.IOException; import java.io.UncheckedIOException; import lombok.extern.slf4j.Slf4j; +import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.security.Credentials; import org.apache.hadoop.security.UserGroupInformation; import org.springframework.beans.factory.annotation.Autowired; @@ -19,23 +20,36 @@ * basis and token file as pointed by HADOOP_TOKEN_FILE_LOCATION is always updated. */ @Slf4j -public class DelegationTokenRefresher { +public class HdfsDelegationTokenRefresher { - @Autowired private FsStorageProvider fsStorageProvider; + @Autowired HdfsStorage hdfsStorage; /** * Schedule credential refresh (hadoop delegation tokens) daily twice. The schedule cron - * expression represented by #{clusterProperties.clusterStorageHadoopTokenRefreshScheduleCron} - * sets the cron to run every 12 hours i.e. daily twice. Hadoop delegation token is valid for 24 - * hours and hence the token must be refreshed before that. The hadoop delegation token file is - * pointed by environment variable i.e. HADOOP_TOKEN_FILE_LOCATION. The renewal of the delegation - * token must be done before it expires. This code assumes that hadoop delegation tokens are - * renewed on a regular basis and token file as pointed by HADOOP_TOKEN_FILE_LOCATION is always - * updated. So, this methods reads the token file and updates the current user - * UserGroupInformation (UGI) with the renewed token and this update is done daily twice. + * expression represented by HdfsStorage specific parameter "token.refresh.schedule.cron" sets the + * cron to run every 12 hours i.e. daily twice. Hadoop delegation token is valid for 24 hours and + * hence the token must be refreshed before that. The hadoop delegation token file is pointed by + * environment variable i.e. HADOOP_TOKEN_FILE_LOCATION. The renewal of the delegation token must + * be done before it expires. This code assumes that hadoop delegation tokens are renewed on a + * regular basis and token file as pointed by HADOOP_TOKEN_FILE_LOCATION is always updated. So, + * this methods reads the token file and updates the current user UserGroupInformation (UGI) with + * the renewed token and this update is done daily twice. The relevant configuration in the + * cluster YAML file is as follows: + * + * 

+   * cluster:
+   *   storages:
+   *     hdfs:
+   *       parameter:
+   *         token.refresh.enabled: true
+   *         token.refresh.schedule.cron: 0 0 0/12 * * ?
+   *
*/ - @Scheduled(cron = "#{clusterProperties.clusterStorageHadoopTokenRefreshScheduleCron}") + @Scheduled( + cron = + "#{hdfsStorage.getProperties().getOrDefault('token.refresh.schedule.cron', '0 0 0/12 * * ?')}") public void refresh() { + log.info("Refreshing HDFS delegation token"); String tokenFileLocation = System.getenv(HADOOP_TOKEN_FILE_LOCATION); try { log.info( @@ -55,9 +69,8 @@ public void refresh() { + HADOOP_TOKEN_FILE_LOCATION + " not found"); } - Credentials cred = - Credentials.readTokenStorageFile( - tokenFile, fsStorageProvider.storageClient().getConf()); + FileSystem fs = (FileSystem) hdfsStorage.getClient().getNativeClient(); + Credentials cred = Credentials.readTokenStorageFile(tokenFile, fs.getConf()); log.info("Loaded {} tokens", cred.numberOfTokens()); UserGroupInformation.getCurrentUser().addCredentials(cred); log.info( diff --git a/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/hdfs/HdfsDelegationTokenRefresherConfig.java b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/hdfs/HdfsDelegationTokenRefresherConfig.java new file mode 100644 index 00000000..42b312a3 --- /dev/null +++ b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/hdfs/HdfsDelegationTokenRefresherConfig.java @@ -0,0 +1,55 @@ +package com.linkedin.openhouse.cluster.storage.hdfs; + +import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.scheduling.annotation.EnableScheduling; + +/** + * Configuration class to conditionally enable/disable creation of {@link + * HdfsDelegationTokenRefresher} bean and enable delegation token refresh scheduling. + */ +@Slf4j +@Configuration +@EnableScheduling +public class HdfsDelegationTokenRefresherConfig { + + @Autowired private HdfsStorage hdfsStorage; + + private static final String HDFS_TOKEN_REFRESH_ENABLED = "token.refresh.enabled"; + + /** + * Conditionally provide the HdfsDelegationTokenRefresher bean if the parameter for token refresh + * is enabled in the HdfsStorage properties. The relevant configuration in the cluster YAML file + * is as follows: + * + * 
+   * cluster:
+   *   storages:
+   *     hdfs:
+   *       parameter:
+   *         token.refresh.enabled: true
+   *
+ * + * @return HdfsDelegationTokenRefresher + */ + @Bean + public HdfsDelegationTokenRefresher getDelegationTokenRefresher() { + if (!hdfsStorage.isConfigured()) { + log.debug( + "Hdfs storage is not configured, ignoring HdfsDelegationTokenRefresher bean creation"); + return null; + } + String refreshEnabled = + hdfsStorage.getProperties().getOrDefault(HDFS_TOKEN_REFRESH_ENABLED, "false"); + if (Boolean.parseBoolean(refreshEnabled)) { + log.info("Creating HdfsDelegationTokenRefresher bean"); + return new HdfsDelegationTokenRefresher(); + } else { + log.debug( + "Hdfs storage token refresh is not enabled, ignoring HdfsDelegationTokenRefresher bean creation"); + return null; + } + } +} diff --git a/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/hdfs/HdfsStorage.java b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/hdfs/HdfsStorage.java new file mode 100644 index 00000000..a6ad210c --- /dev/null +++ b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/hdfs/HdfsStorage.java @@ -0,0 +1,39 @@ +package com.linkedin.openhouse.cluster.storage.hdfs; + +import com.linkedin.openhouse.cluster.storage.BaseStorage; +import com.linkedin.openhouse.cluster.storage.StorageClient; +import com.linkedin.openhouse.cluster.storage.StorageType; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Lazy; +import org.springframework.stereotype.Component; + +/** + * The HdfsStorage class is an implementation of the Storage interface for HDFS storage. It uses a + * HdfsStorageClient to interact with the HDFS file system. The HdfsStorageClient uses the {@link + * org.apache.hadoop.fs.FileSystem} class to interact with the HDFS file system. + */ +@Component +public class HdfsStorage extends BaseStorage { + + @Autowired @Lazy private HdfsStorageClient hdfsStorageClient; + + /** + * Get the type of the HDFS storage. + * + * @return the type of the HDFS storage + */ + @Override + public StorageType.Type getType() { + return StorageType.HDFS; + } + + /** + * Get the HDFS storage client. + * + * @return the HDFS storage client + */ + @Override + public StorageClient getClient() { + return hdfsStorageClient; + } +} diff --git a/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/hdfs/HdfsStorageClient.java b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/hdfs/HdfsStorageClient.java new file mode 100644 index 00000000..b18ddfbf --- /dev/null +++ b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/hdfs/HdfsStorageClient.java @@ -0,0 +1,76 @@ +package com.linkedin.openhouse.cluster.storage.hdfs; + +import com.google.common.base.Preconditions; +import com.linkedin.openhouse.cluster.storage.StorageClient; +import com.linkedin.openhouse.cluster.storage.StorageType; +import com.linkedin.openhouse.cluster.storage.configs.StorageProperties; +import java.io.IOException; +import javax.annotation.PostConstruct; +import lombok.extern.slf4j.Slf4j; +import org.apache.hadoop.fs.FileSystem; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Lazy; +import org.springframework.stereotype.Component; +import org.springframework.util.CollectionUtils; + +/** + * The HdfsStorageClient class is an implementation of the StorageClient interface for HDFS Storage. + * It uses the {@link FileSystem} class to interact with the HDFS file system. + */ +@Slf4j +@Lazy +@Component +public class HdfsStorageClient implements StorageClient { + + private FileSystem fs; + + @Autowired private StorageProperties storageProperties; + + private static final StorageType.Type HDFS_TYPE = StorageType.HDFS; + + /** Initialize the HdfsStorageClient when the bean is accessed for the first time. */ + @PostConstruct + public synchronized void init() throws IOException { + validateProperties(); + StorageProperties.StorageTypeProperties hdfsStorageProperties = + storageProperties.getTypes().get(HDFS_TYPE.getValue()); + org.apache.hadoop.conf.Configuration configuration = new org.apache.hadoop.conf.Configuration(); + configuration.set("fs.defaultFS", hdfsStorageProperties.getEndpoint()); + fs = FileSystem.get(configuration); + } + + /** Validate the storage properties. */ + private void validateProperties() { + log.info("Initializing storage client for type: " + HDFS_TYPE); + Preconditions.checkArgument( + !CollectionUtils.isEmpty(storageProperties.getTypes()) + && storageProperties.getTypes().containsKey(HDFS_TYPE.getValue()), + "Storage properties doesn't contain type: " + HDFS_TYPE.getValue()); + StorageProperties.StorageTypeProperties hdfsStorageProperties = + storageProperties.getTypes().get(HDFS_TYPE.getValue()); + Preconditions.checkArgument( + hdfsStorageProperties != null, + "Storage properties doesn't contain type: " + HDFS_TYPE.getValue()); + Preconditions.checkArgument( + hdfsStorageProperties.getEndpoint() != null, + "Storage properties doesn't contain endpoint for: " + HDFS_TYPE.getValue()); + Preconditions.checkArgument( + hdfsStorageProperties.getRootPath() != null, + "Storage properties doesn't contain rootpath for: " + HDFS_TYPE.getValue()); + } + + @Override + public FileSystem getNativeClient() { + return fs; + } + + @Override + public String getEndpoint() { + return storageProperties.getTypes().get(HDFS_TYPE.getValue()).getEndpoint(); + } + + @Override + public String getRootPrefix() { + return storageProperties.getTypes().get(HDFS_TYPE.getValue()).getRootPath(); + } +} diff --git a/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/local/LocalStorage.java b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/local/LocalStorage.java new file mode 100644 index 00000000..f3ee5e8c --- /dev/null +++ b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/local/LocalStorage.java @@ -0,0 +1,54 @@ +package com.linkedin.openhouse.cluster.storage.local; + +import com.linkedin.openhouse.cluster.storage.BaseStorage; +import com.linkedin.openhouse.cluster.storage.StorageClient; +import com.linkedin.openhouse.cluster.storage.StorageType; +import com.linkedin.openhouse.cluster.storage.configs.StorageProperties; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Lazy; +import org.springframework.stereotype.Component; + +/** + * The LocalStorage class is an implementation of the Storage interface for local storage. It uses a + * LocalStorageClient to interact with the local file system. The LocalStorageClient uses an Apache + * Hadoop FileSystem to interact with the local file system. + */ +@Component +public class LocalStorage extends BaseStorage { + + private static final StorageType.Type LOCAL_TYPE = StorageType.LOCAL; + + @Autowired private StorageProperties storageProperties; + + // Lazy initialization of the LocalStorageClient + @Autowired @Lazy private LocalStorageClient localStorageClient; + + /** + * Check if the local storage is configured. + * + *

The local storage is considered configured if the default type is not set or no types are + * provided or specific "local" type is provided. + * + * @return true if the local storage is configured, false otherwise + */ + @Override + public boolean isConfigured() { + if (storageProperties.getDefaultType() == null) { + return true; + } else if (storageProperties.getTypes() == null || storageProperties.getTypes().isEmpty()) { + return true; + } else { + return storageProperties.getTypes().containsKey(LOCAL_TYPE.getValue()); + } + } + + @Override + public StorageType.Type getType() { + return LOCAL_TYPE; + } + + @Override + public StorageClient getClient() { + return localStorageClient; + } +} diff --git a/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/local/LocalStorageClient.java b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/local/LocalStorageClient.java new file mode 100644 index 00000000..1ca20cb8 --- /dev/null +++ b/cluster/storage/src/main/java/com/linkedin/openhouse/cluster/storage/local/LocalStorageClient.java @@ -0,0 +1,98 @@ +package com.linkedin.openhouse.cluster.storage.local; + +import com.google.common.base.Preconditions; +import com.linkedin.openhouse.cluster.storage.StorageClient; +import com.linkedin.openhouse.cluster.storage.StorageType; +import com.linkedin.openhouse.cluster.storage.configs.StorageProperties; +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import javax.annotation.PostConstruct; +import lombok.extern.slf4j.Slf4j; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.LocalFileSystem; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Lazy; +import org.springframework.stereotype.Component; + +/** + * The LocalStorageClient class is an implementation of the StorageClient interface for local + * storage. It uses an Apache Hadoop FileSystem to interact with the local file system. + */ +@Slf4j +@Lazy +@Component +public class LocalStorageClient implements StorageClient { + + private FileSystem fs; + + private static final StorageType.Type LOCAL_TYPE = StorageType.LOCAL; + + private static final String DEFAULT_ENDPOINT = "file:"; + + private static final String DEFAULT_ROOTPATH = "/tmp"; + + private String endpoint; + + private String rootPath; + + @Autowired private StorageProperties storageProperties; + + /** Initialize the LocalStorageClient when the bean is accessed for the first time. */ + @PostConstruct + public synchronized void init() throws URISyntaxException, IOException { + log.info("Initializing storage client for type: " + LOCAL_TYPE); + + URI uri; + if (storageProperties.getTypes() != null && !storageProperties.getTypes().isEmpty()) { + Preconditions.checkArgument( + storageProperties.getTypes().containsKey(LOCAL_TYPE.getValue()), + "Storage properties doesn't contain type: " + LOCAL_TYPE.getValue()); + Preconditions.checkArgument( + storageProperties.getTypes().get(LOCAL_TYPE.getValue()).getEndpoint() != null, + "Storage properties doesn't contain endpoint for: " + LOCAL_TYPE.getValue()); + Preconditions.checkArgument( + storageProperties.getTypes().get(LOCAL_TYPE.getValue()).getRootPath() != null, + "Storage properties doesn't contain rootpath for: " + LOCAL_TYPE.getValue()); + Preconditions.checkArgument( + storageProperties + .getTypes() + .get(LOCAL_TYPE.getValue()) + .getEndpoint() + .startsWith(DEFAULT_ENDPOINT), + "Storage properties endpoint was misconfigured for: " + LOCAL_TYPE.getValue()); + endpoint = storageProperties.getTypes().get(LOCAL_TYPE.getValue()).getEndpoint(); + rootPath = storageProperties.getTypes().get(LOCAL_TYPE.getValue()).getRootPath(); + } else { + endpoint = DEFAULT_ENDPOINT; + rootPath = DEFAULT_ROOTPATH; + } + try { + uri = new URI(endpoint + rootPath); + } catch (URISyntaxException e) { + throw new IllegalArgumentException( + "Storage properties 'endpoint', 'rootpath' was incorrectly configured for: " + + LOCAL_TYPE.getValue(), + e); + } + this.fs = FileSystem.get(uri, new org.apache.hadoop.conf.Configuration()); + Preconditions.checkArgument( + fs instanceof LocalFileSystem, + "Instantiation failed for LocalStorageClient, fileSystem is not a LocalFileSystem"); + } + + @Override + public FileSystem getNativeClient() { + return fs; + } + + @Override + public String getEndpoint() { + return endpoint; + } + + @Override + public String getRootPrefix() { + return rootPath; + } +} diff --git a/docs/specs/jobs.md b/docs/specs/jobs.md index 6851510a..9250a839 100644 --- a/docs/specs/jobs.md +++ b/docs/specs/jobs.md @@ -206,6 +206,7 @@ Job config |jobType|ORPHAN_FILES_DELETION| |jobType|SNAPSHOTS_EXPIRATION| |jobType|STAGED_FILES_DELETION| +|jobType|ORPHAN_DIRECTORY_DELETION|

JobResponseBody

diff --git a/housetables-service.Dockerfile b/housetables-service.Dockerfile index c29c485b..e8f698e6 100644 --- a/housetables-service.Dockerfile +++ b/housetables-service.Dockerfile @@ -1,4 +1,4 @@ -FROM openjdk:11 +FROM openjdk:23-ea-11-slim ARG USER=openhouse diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalCatalog.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalCatalog.java index 39cd55a8..9c8c2678 100644 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalCatalog.java +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalCatalog.java @@ -3,26 +3,24 @@ import static com.linkedin.openhouse.internal.catalog.InternalCatalogMetricsConstant.METRICS_PREFIX; import com.linkedin.openhouse.cluster.metrics.micrometer.MetricsReporter; -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; +import com.linkedin.openhouse.cluster.storage.StorageManager; +import com.linkedin.openhouse.internal.catalog.fileio.FileIOManager; import com.linkedin.openhouse.internal.catalog.mapper.HouseTableMapper; import com.linkedin.openhouse.internal.catalog.model.HouseTablePrimaryKey; import com.linkedin.openhouse.internal.catalog.repository.HouseTableRepository; import com.linkedin.openhouse.internal.catalog.repository.exception.HouseTableRepositoryException; import io.micrometer.core.instrument.MeterRegistry; -import java.io.IOException; -import java.io.UncheckedIOException; import java.util.List; import java.util.stream.Collectors; import java.util.stream.StreamSupport; import lombok.extern.slf4j.Slf4j; -import org.apache.hadoop.fs.FileSystem; -import org.apache.hadoop.fs.Path; import org.apache.iceberg.BaseMetastoreCatalog; import org.apache.iceberg.TableOperations; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.ValidationException; import org.apache.iceberg.io.FileIO; +import org.apache.iceberg.io.SupportsPrefixOperations; import org.apache.iceberg.relocated.com.google.common.collect.Lists; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @@ -38,7 +36,9 @@ public class OpenHouseInternalCatalog extends BaseMetastoreCatalog { @Autowired HouseTableRepository houseTableRepository; - @Autowired FileIO fileIO; + @Autowired FileIOManager fileIOManager; + + @Autowired StorageManager storageManager; @Autowired SnapshotInspector snapshotInspector; @@ -46,13 +46,11 @@ public class OpenHouseInternalCatalog extends BaseMetastoreCatalog { @Autowired MeterRegistry meterRegistry; - @Autowired FsStorageProvider fsStorageProvider; - @Override protected TableOperations newTableOps(TableIdentifier tableIdentifier) { return new OpenHouseInternalTableOperations( houseTableRepository, - fileIO, + fileIOManager.getFileIO(storageManager.getDefaultStorage().getType()), snapshotInspector, houseTableMapper, tableIdentifier, @@ -89,6 +87,7 @@ public List listTables(Namespace namespace) { @Override public boolean dropTable(TableIdentifier identifier, boolean purge) { String tableLocation = loadTable(identifier).location(); + log.debug("Dropping table {}, purge:{}", tableLocation, purge); try { houseTableRepository.deleteById( HouseTablePrimaryKey.builder() @@ -101,16 +100,17 @@ public boolean dropTable(TableIdentifier identifier, boolean purge) { houseTableRepositoryException); } if (purge) { - // delete data and metadata files on hdfs - try { - FileSystem fs = fsStorageProvider.storageClient(); - fs.delete(new Path(tableLocation), true); - } catch (IOException e) { - throw new UncheckedIOException( - String.format( - "Deleting Directory Failed databaseId: %s, tableId: %s, tableLocation: %s", - identifier.namespace().toString(), identifier.name(), tableLocation), - e); + // Delete data and metadata files from storage. + FileIO fileIO = fileIOManager.getFileIO(storageManager.getDefaultStorage().getType()); + if (fileIO instanceof SupportsPrefixOperations) { + log.debug("Deleting files for table {}", tableLocation); + ((SupportsPrefixOperations) fileIO).deletePrefix(tableLocation); + } else { + log.debug( + "Failed to delete files for table {}. fileIO does not support prefix operations.", + tableLocation); + throw new UnsupportedOperationException( + "Drop table is supported only with a fileIO instance that SupportsPrefixOperations"); } } return true; diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/SnapshotInspector.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/SnapshotInspector.java index 1a619819..dc7dd06c 100644 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/SnapshotInspector.java +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/SnapshotInspector.java @@ -1,6 +1,5 @@ package com.linkedin.openhouse.internal.catalog; -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; import com.linkedin.openhouse.internal.catalog.exception.InvalidIcebergSnapshotException; import java.io.UncheckedIOException; import java.util.List; @@ -28,8 +27,6 @@ */ @Component public class SnapshotInspector { - @Autowired private FsStorageProvider fsStorageProvider; - @Autowired private Consumer> fileSecurer; /** * TODO: ADD Validation for snapshot: Sequence-number based, schema-id based, see iceberg spec for diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/fileio/FileIOConfig.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/fileio/FileIOConfig.java new file mode 100644 index 00000000..0548ab21 --- /dev/null +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/fileio/FileIOConfig.java @@ -0,0 +1,65 @@ +package com.linkedin.openhouse.internal.catalog.fileio; + +import com.linkedin.openhouse.cluster.storage.StorageManager; +import com.linkedin.openhouse.cluster.storage.StorageType; +import lombok.extern.slf4j.Slf4j; +import org.apache.hadoop.fs.FileSystem; +import org.apache.iceberg.hadoop.HadoopFileIO; +import org.apache.iceberg.io.FileIO; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +/** + * Configures the FileIO beans for storages configured in {@link StorageManager} + * + *

Each storage type should have a corresponding FileIO bean defined in this class. The return + * value of the bean is null if the storage type is not configured. The return class of the bean is + * the FileIO implementation for the respective storage type. If conflicting class could be returned + * for the same storage type, the bean name should be annotated with Qualifier to distinguish + * between them. + */ +@Slf4j +@Configuration +public class FileIOConfig { + + @Autowired StorageManager storageManager; + + /** + * Provides the HdfsFileIO bean for HDFS storage type + * + * @return HdfsFileIO bean for HDFS storage type, or null if HDFS storage type is not configured + */ + @Bean("HdfsFileIO") + HadoopFileIO provideHdfsFileIO() { + try { + FileSystem fs = + (FileSystem) storageManager.getStorage(StorageType.HDFS).getClient().getNativeClient(); + return new HadoopFileIO(fs.getConf()); + } catch (IllegalArgumentException e) { + // If the HDFS storage type is not configured, return null + // Spring doesn't define the bean if the return value is null + log.debug("HDFS storage type is not configured", e); + return null; + } + } + + /** + * Provides the HdfsFileIO bean for Local storage type + * + * @return HdfsFileIO bean for Local storage type, or null if Local storage type is not configured + */ + @Bean("LocalFileIO") + FileIO provideLocalFileIO() { + try { + FileSystem fs = + (FileSystem) storageManager.getStorage(StorageType.LOCAL).getClient().getNativeClient(); + return new HadoopFileIO(fs.getConf()); + } catch (IllegalArgumentException e) { + // If the Local storage type is not configured, return null + // Spring doesn't define the bean if the return value is null + log.debug("Local storage type is not configured", e); + return null; + } + } +} diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/fileio/FileIOManager.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/fileio/FileIOManager.java new file mode 100644 index 00000000..987f3fee --- /dev/null +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/fileio/FileIOManager.java @@ -0,0 +1,74 @@ +package com.linkedin.openhouse.internal.catalog.fileio; + +import static com.linkedin.openhouse.cluster.storage.StorageType.HDFS; +import static com.linkedin.openhouse.cluster.storage.StorageType.LOCAL; + +import com.linkedin.openhouse.cluster.storage.Storage; +import com.linkedin.openhouse.cluster.storage.StorageType; +import com.linkedin.openhouse.cluster.storage.hdfs.HdfsStorage; +import com.linkedin.openhouse.cluster.storage.local.LocalStorage; +import java.util.Optional; +import java.util.function.Supplier; +import org.apache.iceberg.hadoop.HadoopFileIO; +import org.apache.iceberg.io.FileIO; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.stereotype.Component; + +/** + * This is the main class that provides the FileIO implementation based on the storage type. Each + * storage type should have a corresponding FileIO bean field defined in this class and the + * corresponding FileIO bean should be returned for appropriate storage type in the method {@link + * #getFileIO(StorageType.Type)}. If the storage type is not configured, the method should throw an + * IllegalArgumentException. + */ +@Component +public class FileIOManager { + + @Autowired(required = false) + @Qualifier("HdfsFileIO") + HadoopFileIO hdfsFileIO; + + @Autowired(required = false) + @Qualifier("LocalFileIO") + FileIO localFileIO; + + @Autowired HdfsStorage hdfsStorage; + + @Autowired LocalStorage localStorage; + /** + * Returns the FileIO implementation for the given storage type. + * + * @param storageType, the storage type for which the FileIO implementation is required + * @return FileIO implementation for the given storage type + * @throws IllegalArgumentException if the storage type is not configured + */ + public FileIO getFileIO(StorageType.Type storageType) throws IllegalArgumentException { + Supplier exceptionSupplier = + () -> new IllegalArgumentException(storageType.getValue() + " is not configured"); + if (HDFS.equals(storageType)) { + return Optional.ofNullable(hdfsFileIO).orElseThrow(exceptionSupplier); + } else if (LOCAL.equals(storageType)) { + return Optional.ofNullable(localFileIO).orElseThrow(exceptionSupplier); + } else { + throw new IllegalArgumentException("FileIO not supported for storage type: " + storageType); + } + } + + /** + * Returns the Storage implementation for the given FileIO. + * + * @param fileIO, the FileIO for which the Storage implementation is required + * @return Storage implementation for the given FileIO + * @throws IllegalArgumentException if the FileIO is not configured + */ + public Storage getStorage(FileIO fileIO) { + if (fileIO.equals(hdfsFileIO)) { + return hdfsStorage; + } else if (fileIO.equals(localFileIO)) { + return localStorage; + } else { + throw new IllegalArgumentException("Storage not supported for fileIO: " + fileIO); + } + } +} diff --git a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/model/HouseTable.java b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/model/HouseTable.java index a63c2a58..f83319fa 100644 --- a/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/model/HouseTable.java +++ b/iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/model/HouseTable.java @@ -1,5 +1,6 @@ package com.linkedin.openhouse.internal.catalog.model; +import com.linkedin.openhouse.cluster.storage.StorageType; import javax.persistence.Entity; import javax.persistence.Id; import javax.persistence.IdClass; @@ -39,4 +40,12 @@ public class HouseTable { private long lastModifiedTime; private long creationTime; + + /** + * This column indicates the storage type used by this table. See {@link + * com.linkedin.openhouse.cluster.storage.StorageType}. A storage type indicates the {@link + * com.linkedin.openhouse.cluster.storage.StorageClient} implementation that is used to interact + * with this table. + */ + @Builder.Default private String storageType = StorageType.HDFS.getValue(); } diff --git a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/MockApplication.java b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/MockApplication.java index 2e88e2e8..480d4303 100644 --- a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/MockApplication.java +++ b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/MockApplication.java @@ -1,6 +1,8 @@ package com.linkedin.openhouse.internal.catalog; -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; +import com.linkedin.openhouse.cluster.storage.StorageManager; +import com.linkedin.openhouse.internal.catalog.fileio.FileIOConfig; +import com.linkedin.openhouse.internal.catalog.fileio.FileIOManager; import java.io.IOException; import java.io.UncheckedIOException; import java.util.function.Consumer; @@ -11,7 +13,6 @@ import org.apache.hadoop.fs.permission.FsAction; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.iceberg.catalog.Catalog; -import org.apache.iceberg.io.FileIO; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; import org.springframework.boot.test.mock.mockito.MockBean; @@ -25,9 +26,11 @@ public static void main(String[] args) { @MockBean Catalog openHouseInternalCatalog; - @MockBean FileIO fileIO; + @MockBean StorageManager storageManager; - @MockBean FsStorageProvider fsStorageProvider; + @MockBean FileIOManager fileIOManager; + + @MockBean FileIOConfig fileIOConfig; static final FsPermission perm = new FsPermission(FsAction.ALL, FsAction.NONE, FsAction.NONE); diff --git a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/fileio/FileIOManagerTest.java b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/fileio/FileIOManagerTest.java new file mode 100644 index 00000000..17cd0fcf --- /dev/null +++ b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/fileio/FileIOManagerTest.java @@ -0,0 +1,37 @@ +package com.linkedin.openhouse.internal.catalog.fileio; + +import com.linkedin.openhouse.cluster.storage.StorageType; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.context.annotation.ComponentScan; +import org.springframework.context.annotation.Configuration; + +@SpringBootTest(classes = FileIOManagerTest.FileIOManagerTestConfig.class) +public class FileIOManagerTest { + + @Autowired FileIOManager fileIOManager; + + @Test + public void testGetLocalFileIO() { + // local storage is configured + Assertions.assertNotNull(fileIOManager.getFileIO(StorageType.LOCAL)); + } + + @Test + public void testGetUndefinedFileIOThrowsException() { + // hdfs storage is not configured + Assertions.assertThrows( + IllegalArgumentException.class, () -> fileIOManager.getFileIO(StorageType.HDFS)); + } + + @Configuration + @ComponentScan( + basePackages = { + "com.linkedin.openhouse.internal.catalog.fileio", + "com.linkedin.openhouse.cluster.storage", + "com.linkedin.openhouse.cluster.configs" + }) + public static class FileIOManagerTestConfig {} +} diff --git a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/model/HouseTableTest.java b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/model/HouseTableTest.java new file mode 100644 index 00000000..d7a82829 --- /dev/null +++ b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/model/HouseTableTest.java @@ -0,0 +1,51 @@ +package com.linkedin.openhouse.internal.catalog.model; + +import com.linkedin.openhouse.cluster.storage.StorageType; +import com.linkedin.openhouse.internal.catalog.mapper.HouseTableSerdeUtils; +import java.beans.BeanInfo; +import java.beans.Introspector; +import java.beans.PropertyDescriptor; +import java.lang.reflect.Method; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class HouseTableTest { + + @Test + public void testHouseTableDefaultValues() { + HouseTable ht = HouseTable.builder().build(); + + try { + BeanInfo beanInfo = Introspector.getBeanInfo(HouseTable.class); + + PropertyDescriptor[] propertyDescriptors = beanInfo.getPropertyDescriptors(); + + for (PropertyDescriptor propertyDescriptor : propertyDescriptors) { + + if (propertyDescriptor.getReadMethod() != null) { + Method getter = propertyDescriptor.getReadMethod(); + Object value = getter.invoke(ht); + + String fieldName = + getter.getName().substring(3, 4).toLowerCase() + getter.getName().substring(4); + if (!HouseTableSerdeUtils.HTS_FIELD_NAMES.contains(fieldName)) { + // not a field getter + continue; + } + + if (fieldName.equals("storageType")) { + Assertions.assertEquals(StorageType.HDFS.getValue(), value); + } else if (fieldName.equals("creationTime")) { + Assertions.assertEquals(0L, value); + } else if (fieldName.equals("lastModifiedTime")) { + Assertions.assertEquals(0L, value); + } else { + Assertions.assertNull(value, getter.getName() + " is not null: " + value); + } + } + } + } catch (Exception e) { + Assertions.fail(e); + } + } +} diff --git a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/repository/HouseTableRepositoryImplTest.java b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/repository/HouseTableRepositoryImplTest.java index 4f644566..7be9063e 100644 --- a/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/repository/HouseTableRepositoryImplTest.java +++ b/iceberg/openhouse/internalcatalog/src/test/java/com/linkedin/openhouse/internal/catalog/repository/HouseTableRepositoryImplTest.java @@ -132,6 +132,7 @@ public void testRepoSave() { Assertions.assertEquals(result.getDatabaseId(), HOUSE_TABLE.getDatabaseId()); Assertions.assertEquals(result.getTableLocation(), HOUSE_TABLE.getTableLocation()); Assertions.assertEquals(result.getTableVersion(), HOUSE_TABLE.getTableVersion()); + Assertions.assertEquals(result.getStorageType(), HOUSE_TABLE.getStorageType()); } @Test diff --git a/infra/recipes/docker-compose/common/spark/spark-base-hadoop3.2.dockerfile b/infra/recipes/docker-compose/common/spark/spark-base-hadoop3.2.dockerfile index 1f405483..dfbd1dc6 100644 --- a/infra/recipes/docker-compose/common/spark/spark-base-hadoop3.2.dockerfile +++ b/infra/recipes/docker-compose/common/spark/spark-base-hadoop3.2.dockerfile @@ -81,7 +81,7 @@ RUN mkdir -p "${LIVY_HOME}/logs" COPY /infra/recipes/docker-compose/common/spark/start-spark.sh / COPY /build/openhouse-spark-runtime_2.12/libs/*[^sources][^javadoc].jar $SPARK_HOME/openhouse-spark-runtime_2.12-latest-all.jar -COPY /build/openhouse-spark-apps_2.12/libs/openhouse-spark-apps_2.12-all.jar $SPARK_HOME/openhouse-spark-apps_2.12-latest-all.jar +COPY /build/openhouse-spark-apps_2.12/libs/openhouse-spark-apps_2.12-uber.jar $SPARK_HOME/openhouse-spark-apps_2.12-latest-all.jar COPY /build/dummytokens/libs/dummytokens*.jar /dummytokens.jar RUN java -jar /dummytokens.jar -d /var/config/ diff --git a/infra/recipes/docker-compose/oh-hadoop-spark/cluster.yaml b/infra/recipes/docker-compose/oh-hadoop-spark/cluster.yaml index 2bee0714..bb72176c 100644 --- a/infra/recipes/docker-compose/oh-hadoop-spark/cluster.yaml +++ b/infra/recipes/docker-compose/oh-hadoop-spark/cluster.yaml @@ -4,6 +4,12 @@ cluster: type: "hadoop" uri: "hdfs://namenode:9000/" root-path: "/data/openhouse" + storages: + default-type: "hdfs" + types: + hdfs: + rootpath: "/data/openhouse" + endpoint: "hdfs://namenode:9000/" iceberg: write: format: diff --git a/infra/recipes/docker-compose/oh-hadoop-spark/jobs.yaml b/infra/recipes/docker-compose/oh-hadoop-spark/jobs.yaml index edc896a8..926ddab7 100644 --- a/infra/recipes/docker-compose/oh-hadoop-spark/jobs.yaml +++ b/infra/recipes/docker-compose/oh-hadoop-spark/jobs.yaml @@ -50,7 +50,7 @@ jobs: args: ["--trashDir", ".trash", "--daysOld", "10", "--recursive", "true"] <<: *apps-defaults - type: ORPHAN_DIRECTORY_DELETION - class-name: com.linkedin.openhouse.jobs.spark.OrphanDirectoryDeletionSparkApp + class-name: com.linkedin.openhouse.jobs.spark.OrphanTableDirectoryDeletionSparkApp args: [ "--trashDir", ".trash" ] <<: *apps-defaults - type: TABLE_STATS_COLLECTION diff --git a/infra/recipes/docker-compose/oh-hadoop/cluster.yaml b/infra/recipes/docker-compose/oh-hadoop/cluster.yaml index 5d870a7a..75e18624 100644 --- a/infra/recipes/docker-compose/oh-hadoop/cluster.yaml +++ b/infra/recipes/docker-compose/oh-hadoop/cluster.yaml @@ -4,6 +4,12 @@ cluster: type: "hadoop" uri: "hdfs://namenode:9000/" root-path: "/data/openhouse" + storages: + default-type: "hdfs" + types: + hdfs: + rootpath: "/data/openhouse" + endpoint: "hdfs://namenode:9000/" iceberg: write: format: diff --git a/integrations/java/openhouse-java-itest/src/test/java/com/linkedin/openhouse/javaclient/OpenHouseTableOperationsTest.java b/integrations/java/openhouse-java-itest/src/test/java/com/linkedin/openhouse/javaclient/OpenHouseTableOperationsTest.java index bed21717..884eeffe 100644 --- a/integrations/java/openhouse-java-itest/src/test/java/com/linkedin/openhouse/javaclient/OpenHouseTableOperationsTest.java +++ b/integrations/java/openhouse-java-itest/src/test/java/com/linkedin/openhouse/javaclient/OpenHouseTableOperationsTest.java @@ -87,6 +87,7 @@ public void testCreateUpdateTableErrorHandle() { when(base.snapshots()).thenReturn(snapshotList); // Ensure tableApi throw expected exception + when(mockTableApi.updateTableV1(anyString(), anyString(), any())) .thenReturn(Mono.error(mock(WebClientResponseException.ServiceUnavailable.class))); Assertions.assertThrows( @@ -99,6 +100,10 @@ public void testCreateUpdateTableErrorHandle() { .thenReturn(Mono.error(mock(WebClientResponseException.NotFound.class))); Assertions.assertThrows( NoSuchTableException.class, () -> openHouseTableOperations.doCommit(base, metadata)); + when(mockTableApi.updateTableV1(anyString(), anyString(), any())) + .thenReturn(Mono.error(mock(WebClientResponseException.InternalServerError.class))); + Assertions.assertThrows( + CommitStateUnknownException.class, () -> openHouseTableOperations.doCommit(base, metadata)); when(mockTableApi.updateTableV1(anyString(), anyString(), any())) .thenReturn(Mono.error(mock(WebClientRequestException.class))); Assertions.assertThrows( diff --git a/integrations/java/openhouse-java-runtime/src/main/java/com/linkedin/openhouse/javaclient/OpenHouseTableOperations.java b/integrations/java/openhouse-java-runtime/src/main/java/com/linkedin/openhouse/javaclient/OpenHouseTableOperations.java index 82503d95..be0aea77 100644 --- a/integrations/java/openhouse-java-runtime/src/main/java/com/linkedin/openhouse/javaclient/OpenHouseTableOperations.java +++ b/integrations/java/openhouse-java-runtime/src/main/java/com/linkedin/openhouse/javaclient/OpenHouseTableOperations.java @@ -276,9 +276,16 @@ static Mono handleCreateUpdateHttpError( return Mono.error( new CommitFailedException( casted, casted.getStatusCode().value() + " , " + casted.getResponseBodyAsString())); + } else if (e instanceof WebClientResponseException.GatewayTimeout - | e instanceof WebClientResponseException.ServiceUnavailable) { - return Mono.error(new CommitStateUnknownException(e)); + || e instanceof WebClientResponseException.ServiceUnavailable + || e instanceof WebClientResponseException.InternalServerError) { + /** + * This is done to avoid any data loss that could occur when a commit aborts at the caller + * leads to deletion of iceberg metadata files. + */ + WebClientResponseException casted = (WebClientResponseException) e; + return Mono.error(new CommitStateUnknownException(casted.getResponseBodyAsString(), casted)); } else if (e instanceof WebClientResponseException.BadRequest) { WebClientResponseException casted = (WebClientResponseException) e; return Mono.error( @@ -288,9 +295,10 @@ static Mono handleCreateUpdateHttpError( return Mono.error(new WebClientResponseWithMessageException((WebClientResponseException) e)); } else { /** - * This serves as a catch-all for any other exceptions that are not - * WebClientResponseException. It helps in skipping any unexpected cleanup that could occur - * when a commit aborts at the caller, thus avoiding any potential data loss. + * This serves as a catch-all for any unexpected exceptions that could occur during doCommit, + * (i.e) exceptions that are not WebClientResponseException. This is a conservative approach + * to skip any unexpected cleanup that could occur when a commit aborts at the caller, thus + * avoiding any potential data loss. */ return Mono.error(new CommitStateUnknownException(e)); } diff --git a/integrations/spark/openhouse-spark-itest/src/test/java/com/linkedin/openhouse/spark/mock/DoCommitTest.java b/integrations/spark/openhouse-spark-itest/src/test/java/com/linkedin/openhouse/spark/mock/DoCommitTest.java index 75822109..9dcf4d56 100644 --- a/integrations/spark/openhouse-spark-itest/src/test/java/com/linkedin/openhouse/spark/mock/DoCommitTest.java +++ b/integrations/spark/openhouse-spark-itest/src/test/java/com/linkedin/openhouse/spark/mock/DoCommitTest.java @@ -4,7 +4,6 @@ import static com.linkedin.openhouse.spark.SparkTestBase.*; import com.linkedin.openhouse.javaclient.OpenHouseTableOperations; -import com.linkedin.openhouse.relocated.org.springframework.web.reactive.function.client.WebClientResponseException; import com.linkedin.openhouse.spark.SparkTestBase; import java.nio.file.Files; import java.nio.file.Path; @@ -133,7 +132,7 @@ public void testCommitFailedException() { @Test public void testSurfaceRestExceptions() { mockTableService.enqueue(mockResponse(500, "{\"message\":\"Internal Server Error\"}")); - Assertions.assertThrows(WebClientResponseException.class, () -> ops.doCommit(null, base)); + Assertions.assertThrows(CommitStateUnknownException.class, () -> ops.doCommit(null, base)); } @Test diff --git a/integrations/spark/openhouse-spark-runtime/build.gradle b/integrations/spark/openhouse-spark-runtime/build.gradle index 451c49cb..24f3932b 100644 --- a/integrations/spark/openhouse-spark-runtime/build.gradle +++ b/integrations/spark/openhouse-spark-runtime/build.gradle @@ -45,6 +45,14 @@ dependencies { exclude group: 'org.roaringbitmap' } + testImplementation("org.apache.iceberg:iceberg-spark-runtime-3.1_2.12:" + icebergVersion) + testImplementation(project(':tables-test-fixtures_2.12')) + testImplementation('org.apache.spark:spark-sql_2.12:' + spark_version){ + // These classes are available from `client-codegen-convention.gradle` + exclude group: "io.netty" + } + testImplementation(project(path: ':integrations:java:openhouse-java-runtime', configuration: 'shadow')) + fatJarPackagedDependencies(project(path: ':integrations:java:openhouse-java-runtime')) fatJarRuntimeDependencies("org.apache.iceberg:iceberg-spark-runtime-3.1_2.12:" + icebergVersion) diff --git a/integrations/spark/openhouse-spark-runtime/src/test/java/CatalogOperationTest.java b/integrations/spark/openhouse-spark-runtime/src/test/java/CatalogOperationTest.java new file mode 100644 index 00000000..d1f88127 --- /dev/null +++ b/integrations/spark/openhouse-spark-runtime/src/test/java/CatalogOperationTest.java @@ -0,0 +1,91 @@ +import com.linkedin.openhouse.tablestest.OpenHouseSparkITest; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; +import org.apache.iceberg.CatalogUtil; +import org.apache.iceberg.DataFile; +import org.apache.iceberg.DataFiles; +import org.apache.iceberg.PartitionSpec; +import org.apache.iceberg.Schema; +import org.apache.iceberg.Table; +import org.apache.iceberg.catalog.Catalog; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.types.Types; +import org.apache.spark.sql.SparkSession; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import scala.collection.JavaConverters; + +public class CatalogOperationTest extends OpenHouseSparkITest { + @Test + public void testCasingWithCTAS() throws Exception { + try (SparkSession spark = getSparkSession()) { + // creating a casing preserving table using backtick + spark.sql("CREATE TABLE openhouse.d1.`tT1` (name string)"); + // testing writing behavior, note the casing of tt1 is intentionally changed. + spark.sql("INSERT INTO openhouse.d1.Tt1 VALUES ('foo')"); + + // Verifying by querying with all lower-cased name + Assertions.assertEquals( + 1, spark.sql("SELECT * from openhouse.d1.tt1").collectAsList().size()); + // ctas but referring with lower-cased name + spark.sql("CREATE TABLE openhouse.d1.t2 AS SELECT * from openhouse.d1.tt1"); + Assertions.assertEquals(1, spark.sql("SELECT * FROM openhouse.d1.t2").collectAsList().size()); + } + } + + @Test + public void testCatalogWriteAPI() throws Exception { + try (SparkSession spark = getSparkSession()) { + Catalog icebergCatalog = getOpenHouseCatalog(spark); + // Create a table + Schema schema = new Schema(Types.NestedField.required(1, "name", Types.StringType.get())); + TableIdentifier tableIdentifier = TableIdentifier.of("db", "aaa"); + icebergCatalog.createTable(tableIdentifier, schema); + + // Write into data with intentionally changed casing in name + TableIdentifier tableIdentifierUpperTblName = TableIdentifier.of("db", "AAA"); + + DataFile fooDataFile = + DataFiles.builder(PartitionSpec.unpartitioned()) + .withPath("/path/to/data-a.parquet") + .withFileSizeInBytes(10) + .withRecordCount(1) + .build(); + AtomicReference tableRef = new AtomicReference<>(); + Assertions.assertDoesNotThrow( + () -> { + Table loadedTable = icebergCatalog.loadTable(tableIdentifierUpperTblName); + tableRef.set(loadedTable); + }); + Table table = tableRef.get(); + Assertions.assertDoesNotThrow( + () -> { + table.newAppend().appendFile(fooDataFile).commit(); + }); + } + } + + /** + * This is a copy of com.linkedin.openhouse.jobs.spark.Operations#getCatalog() temporarily. + * Refactoring these pieces require deployment coordination, thus we shall create an artifact + * module that can be pulled by :apps module. + */ + private Catalog getOpenHouseCatalog(SparkSession spark) { + final Map catalogProperties = new HashMap<>(); + final String catalogPropertyPrefix = String.format("spark.sql.catalog.openhouse."); + final Map sparkProperties = JavaConverters.mapAsJavaMap(spark.conf().getAll()); + for (Map.Entry entry : sparkProperties.entrySet()) { + if (entry.getKey().startsWith(catalogPropertyPrefix)) { + catalogProperties.put( + entry.getKey().substring(catalogPropertyPrefix.length()), entry.getValue()); + } + } + // this initializes the catalog based on runtime Catalog class passed in catalog-impl conf. + return CatalogUtil.loadCatalog( + sparkProperties.get("spark.sql.catalog.openhouse.catalog-impl"), + "openhouse", + catalogProperties, + spark.sparkContext().hadoopConfiguration()); + } +} diff --git a/jobs-scheduler.Dockerfile b/jobs-scheduler.Dockerfile index bfa97e53..7ae58633 100644 --- a/jobs-scheduler.Dockerfile +++ b/jobs-scheduler.Dockerfile @@ -1,4 +1,4 @@ -FROM openjdk:11 +FROM openjdk:23-ea-11-slim ARG USER=openhouse ARG USER_ID=1000 @@ -15,7 +15,7 @@ WORKDIR $USER_HOME ENV PATH=$PATH:/export/apps/jdk/JDK-1_8_0_172/bin/:$USER_HOME ARG BUILD_DIR="build/$APP_NAME/libs" -ARG JAR_FILES=$BUILD_DIR/*-all.jar +ARG JAR_FILES=$BUILD_DIR/*-uber.jar COPY $JAR_FILES ./ diff --git a/jobs-service.Dockerfile b/jobs-service.Dockerfile index efa4b8d9..050e51b2 100644 --- a/jobs-service.Dockerfile +++ b/jobs-service.Dockerfile @@ -1,4 +1,4 @@ -FROM openjdk:11 +FROM openjdk:23-ea-11-slim ARG USER=openhouse ARG USER_ID=1000 diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/handler/OpenHouseToggleStatusesApiHandler.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/handler/OpenHouseToggleStatusesApiHandler.java new file mode 100644 index 00000000..b47af11c --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/handler/OpenHouseToggleStatusesApiHandler.java @@ -0,0 +1,48 @@ +package com.linkedin.openhouse.housetables.api.handler; + +import com.linkedin.openhouse.common.api.spec.ApiResponse; +import com.linkedin.openhouse.housetables.api.spec.model.TableToggleStatusKey; +import com.linkedin.openhouse.housetables.api.spec.model.ToggleStatus; +import com.linkedin.openhouse.housetables.api.spec.response.EntityResponseBody; +import com.linkedin.openhouse.housetables.api.spec.response.GetAllEntityResponseBody; +import com.linkedin.openhouse.housetables.services.ToggleStatusesService; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatus; +import org.springframework.stereotype.Component; + +/** + * {@link ToggleStatusesApiHandler} is essentially read only. Thus, any write API are not + * implemented here. + */ +@Component +public class OpenHouseToggleStatusesApiHandler implements ToggleStatusesApiHandler { + @Autowired private ToggleStatusesService toggleStatusesService; + + @Override + public ApiResponse> getEntity(TableToggleStatusKey key) { + return ApiResponse.>builder() + .httpStatus(HttpStatus.OK) + .responseBody( + EntityResponseBody.builder() + .entity( + toggleStatusesService.getTableToggleStatus( + key.getFeatureId(), key.getDatabaseId(), key.getTableId())) + .build()) + .build(); + } + + @Override + public ApiResponse> getEntities(ToggleStatus entity) { + throw new UnsupportedOperationException("Get all toggle status is unsupported"); + } + + @Override + public ApiResponse deleteEntity(TableToggleStatusKey key) { + throw new UnsupportedOperationException("Delete toggle status is unsupported"); + } + + @Override + public ApiResponse> putEntity(ToggleStatus entity) { + throw new UnsupportedOperationException("Update toggle status is unsupported"); + } +} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/handler/ToggleStatusesApiHandler.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/handler/ToggleStatusesApiHandler.java new file mode 100644 index 00000000..1ceb8409 --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/handler/ToggleStatusesApiHandler.java @@ -0,0 +1,7 @@ +package com.linkedin.openhouse.housetables.api.handler; + +import com.linkedin.openhouse.housetables.api.spec.model.TableToggleStatusKey; +import com.linkedin.openhouse.housetables.api.spec.model.ToggleStatus; + +public interface ToggleStatusesApiHandler + extends HouseTablesApiHandler {} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/TableToggleStatusKey.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/TableToggleStatusKey.java new file mode 100644 index 00000000..fa0d0197 --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/TableToggleStatusKey.java @@ -0,0 +1,40 @@ +package com.linkedin.openhouse.housetables.api.spec.model; + +import static com.linkedin.openhouse.common.api.validator.ValidatorConstants.*; + +import com.fasterxml.jackson.annotation.JsonProperty; +import io.swagger.v3.oas.annotations.media.Schema; +import javax.validation.constraints.NotEmpty; +import javax.validation.constraints.Pattern; +import lombok.Builder; +import lombok.Value; + +/** Key to query feature-toggle status of a table. */ +@Builder +@Value +public class TableToggleStatusKey { + @Schema( + description = + "Unique Resource identifier for the Database containing the Table. Together with tableID" + + " they form a composite primary key for a user table.", + example = "my_database") + @JsonProperty(value = "databaseId") + @NotEmpty(message = "databaseId cannot be empty") + @Pattern(regexp = ALPHA_NUM_UNDERSCORE_REGEX, message = ALPHA_NUM_UNDERSCORE_ERROR_MSG) + String databaseId; + + @Schema( + description = "Unique Resource identifier for a table within a Database.", + example = "my_table") + @JsonProperty(value = "tableId") + @NotEmpty(message = "tableId cannot be empty") + @Pattern(regexp = ALPHA_NUM_UNDERSCORE_REGEX, message = ALPHA_NUM_UNDERSCORE_ERROR_MSG) + String tableId; + + @Schema( + description = "Unique Resource identifier for a feature within OpenHouse Service", + example = "wap-branch") + @JsonProperty(value = "featureId") + @NotEmpty(message = "featureId cannot be empty") + String featureId; +} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/ToggleStatus.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/ToggleStatus.java new file mode 100644 index 00000000..2f1ba5c0 --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/ToggleStatus.java @@ -0,0 +1,19 @@ +package com.linkedin.openhouse.housetables.api.spec.model; + +import com.fasterxml.jackson.annotation.JsonProperty; +import io.swagger.v3.oas.annotations.media.Schema; +import javax.validation.constraints.NotEmpty; +import lombok.Builder; +import lombok.Value; + +/** This layer on top of {@link ToggleStatusEnum} is ensuring API extensibility. */ +@Builder(toBuilder = true) +@Value +public class ToggleStatus { + @Schema( + description = "Status of an entity with respect to whether a feature has been toggled on", + example = "Active") + @JsonProperty(value = "status") + @NotEmpty(message = "Toggle status cannot be empty") + ToggleStatusEnum status; +} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/ToggleStatusEnum.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/ToggleStatusEnum.java new file mode 100644 index 00000000..a1ecdeab --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/ToggleStatusEnum.java @@ -0,0 +1,7 @@ +package com.linkedin.openhouse.housetables.api.spec.model; + +/** Indicate if a feature is active or inactive on an entity (e.g. table) */ +public enum ToggleStatusEnum { + ACTIVE, + INACTIVE +} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/UserTable.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/UserTable.java index 2e5c56a6..a46b2bda 100644 --- a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/UserTable.java +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/UserTable.java @@ -37,7 +37,9 @@ public class UserTable { @Pattern(regexp = ALPHA_NUM_UNDERSCORE_REGEX, message = ALPHA_NUM_UNDERSCORE_ERROR_MSG) private String databaseId; - @Schema(description = "Current Version of the user table.", example = "") + @Schema( + description = "Current Version of the user table. New record should have 'INTITAL_VERISON'", + example = "") @JsonProperty(value = "tableVersion") private String tableVersion; diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/UserTableKey.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/UserTableKey.java index ab8685ca..e2c933bf 100644 --- a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/UserTableKey.java +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/UserTableKey.java @@ -29,7 +29,7 @@ public class UserTableKey { + " they form a composite primary key for a user table.", example = "my_database") @JsonProperty(value = "databaseId") - @NotEmpty(message = "tableId cannot be empty") + @NotEmpty(message = "databaseId cannot be empty") @Pattern(regexp = ALPHA_NUM_UNDERSCORE_REGEX, message = ALPHA_NUM_UNDERSCORE_ERROR_MSG) private String databaseId; diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/config/db/iceberg/IcebergProviderConfiguration.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/config/db/iceberg/IcebergProviderConfiguration.java index 496d0e5c..6857b84d 100644 --- a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/config/db/iceberg/IcebergProviderConfiguration.java +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/config/db/iceberg/IcebergProviderConfiguration.java @@ -1,6 +1,7 @@ package com.linkedin.openhouse.housetables.config.db.iceberg; -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; +import com.linkedin.openhouse.cluster.storage.StorageManager; +import com.linkedin.openhouse.cluster.storage.StorageType; import com.linkedin.openhouse.housetables.repository.HtsRepository; import com.linkedin.openhouse.hts.catalog.model.jobtable.JobIcebergRow; import com.linkedin.openhouse.hts.catalog.model.jobtable.JobIcebergRowPrimaryKey; @@ -10,6 +11,7 @@ import java.nio.file.Paths; import java.util.Collections; import lombok.extern.slf4j.Slf4j; +import org.apache.hadoop.fs.FileSystem; import org.apache.iceberg.CatalogProperties; import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.catalog.TableIdentifier; @@ -34,7 +36,7 @@ public class IcebergProviderConfiguration { private static final String HTS_USER_TBL_NAME = "userTable"; private static final String HTS_JOB_TBL_NAME = "jobTable"; - @Autowired FsStorageProvider storageProvider; + @Autowired StorageManager storageManager; private Catalog provideHadoopCatalogForHouseTables() { HadoopCatalog catalog = new HadoopCatalog(); @@ -43,7 +45,7 @@ private Catalog provideHadoopCatalogForHouseTables() { HTS_CATALOG_NAME, Collections.singletonMap( CatalogProperties.WAREHOUSE_LOCATION, - Paths.get(storageProvider.rootPath()).toString())); + Paths.get(storageManager.getDefaultStorage().getClient().getRootPrefix()).toString())); return catalog; } @@ -70,7 +72,15 @@ private Catalog provideHadoopCatalogForHouseTables() { } private org.apache.hadoop.conf.Configuration getHadoopConfigurations() { - log.debug("Loading hadoop configuration from:" + storageProvider.name()); - return storageProvider.storageClient().getConf(); + log.debug("Loading hadoop configuration for:" + storageManager.getDefaultStorage().getType()); + if (storageManager.getDefaultStorage().getType().equals(StorageType.HDFS) + || storageManager.getDefaultStorage().getType().equals(StorageType.LOCAL)) { + return ((FileSystem) storageManager.getDefaultStorage().getClient().getNativeClient()) + .getConf(); + } else { + throw new UnsupportedOperationException( + "Unsupported storage type for Iceberg catalog: " + + storageManager.getDefaultStorage().getType()); + } } } diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/config/db/jdbc/JdbcProviderConfiguration.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/config/db/jdbc/JdbcProviderConfiguration.java index fb2598b2..225dd5d7 100644 --- a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/config/db/jdbc/JdbcProviderConfiguration.java +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/config/db/jdbc/JdbcProviderConfiguration.java @@ -30,7 +30,7 @@ public class JdbcProviderConfiguration { * "htsdb" database. With DB_CLOSE_DELAY=-1, the database is kept alive as long as the JVM lives, * otherwise it shuts down when the database-creating-thread dies. */ - private static final String H2_DEFAULT_URL = "jdbc:h2:mem:htsdb;DB_CLOSE_DELAY=-1"; + private static final String H2_DEFAULT_URL = "jdbc:h2:mem:htsdb;MODE=MySQL;DB_CLOSE_DELAY=-1"; @Bean public DataSource provideJdbcDataSource() { diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/controller/ToggleStatusesController.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/controller/ToggleStatusesController.java new file mode 100644 index 00000000..2dc72075 --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/controller/ToggleStatusesController.java @@ -0,0 +1,56 @@ +package com.linkedin.openhouse.housetables.controller; + +import com.linkedin.openhouse.housetables.api.handler.ToggleStatusesApiHandler; +import com.linkedin.openhouse.housetables.api.spec.model.TableToggleStatusKey; +import com.linkedin.openhouse.housetables.api.spec.model.ToggleStatus; +import com.linkedin.openhouse.housetables.api.spec.response.EntityResponseBody; +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import io.swagger.v3.oas.annotations.responses.ApiResponses; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RestController; + +/** + * Toggle Statuses are read-only for HTS, thus create/update paths are intentionally not + * implemented. + */ +@RestController +public class ToggleStatusesController { + + private static final String TOGGLE_ENDPOINT = "/hts/togglestatuses"; + + @Autowired private ToggleStatusesApiHandler toggleStatuesApiHandler; + + @Operation( + summary = "Get a toggle status applied to a table.", + description = "Returns a toggle status of databaseID and tableId on a featureId", + tags = {"ToggleStatus"}) + @ApiResponses( + value = { + @ApiResponse(responseCode = "200", description = "Toggle status GET: OK"), + @ApiResponse(responseCode = "404", description = "Toggle status GET: NOT FOUND") + }) + @GetMapping( + value = TOGGLE_ENDPOINT, + produces = {"application/json"}) + public ResponseEntity> getTableToggleStatus( + @RequestParam(value = "databaseId") String databaseId, + @RequestParam(value = "tableId") String tableId, + @RequestParam(value = "featureId") String featureId) { + + com.linkedin.openhouse.common.api.spec.ApiResponse> + apiResponse = + toggleStatuesApiHandler.getEntity( + TableToggleStatusKey.builder() + .databaseId(databaseId) + .tableId(tableId) + .featureId(featureId) + .build()); + + return new ResponseEntity<>( + apiResponse.getResponseBody(), apiResponse.getHttpHeaders(), apiResponse.getHttpStatus()); + } +} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/dto/mapper/UserTableVersionMapper.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/dto/mapper/UserTableVersionMapper.java index 09300f14..bda09d2b 100644 --- a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/dto/mapper/UserTableVersionMapper.java +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/dto/mapper/UserTableVersionMapper.java @@ -1,5 +1,6 @@ package com.linkedin.openhouse.housetables.dto.mapper; +import com.linkedin.openhouse.common.api.validator.ValidatorConstants; import com.linkedin.openhouse.common.exception.EntityConcurrentModificationException; import com.linkedin.openhouse.housetables.api.spec.model.UserTable; import com.linkedin.openhouse.housetables.model.UserTableRow; @@ -19,7 +20,16 @@ public class UserTableVersionMapper { @Named("toVersion") public Long toVersion(UserTable userTable, @Context Optional existingUserTableRow) { if (!existingUserTableRow.isPresent()) { - return 1L; + if (!userTable.getTableVersion().equals(ValidatorConstants.INITIAL_TABLE_VERSION)) { + throw new EntityConcurrentModificationException( + String.format( + "databaseId : %s, tableId : %s %s", + userTable.getDatabaseId(), + userTable.getTableId(), + "The requested user table has been deleted by other processes."), + new RuntimeException()); + } + return null; } else { if (existingUserTableRow.get().getMetadataLocation().equals(userTable.getTableVersion())) { return existingUserTableRow.get().getVersion(); diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/model/TableToggleRule.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/model/TableToggleRule.java new file mode 100644 index 00000000..cf1cedfc --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/model/TableToggleRule.java @@ -0,0 +1,32 @@ +package com.linkedin.openhouse.housetables.model; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import lombok.AccessLevel; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.NoArgsConstructor; + +/** Data Model for persisting a Toggle Rule Object in the HouseTable. */ +@Entity +@Builder(toBuilder = true) +@Getter +@EqualsAndHashCode +@NoArgsConstructor(access = AccessLevel.PROTECTED) +@AllArgsConstructor(access = AccessLevel.PROTECTED) +public class TableToggleRule { + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + @Column(name = "id", updatable = false, nullable = false) + private Long id; + + private String feature; + private String databasePattern; + private String tablePattern; + private Long creationTimeMs; +} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/repository/impl/jdbc/ToggleStatusHtsJdbcRepository.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/repository/impl/jdbc/ToggleStatusHtsJdbcRepository.java new file mode 100644 index 00000000..944a7917 --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/repository/impl/jdbc/ToggleStatusHtsJdbcRepository.java @@ -0,0 +1,8 @@ +package com.linkedin.openhouse.housetables.repository.impl.jdbc; + +import com.linkedin.openhouse.housetables.model.TableToggleRule; +import com.linkedin.openhouse.housetables.repository.HtsRepository; + +public interface ToggleStatusHtsJdbcRepository extends HtsRepository { + Iterable findAllByFeature(String feature); +} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesService.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesService.java new file mode 100644 index 00000000..e5b498c2 --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesService.java @@ -0,0 +1,16 @@ +package com.linkedin.openhouse.housetables.services; + +import com.linkedin.openhouse.housetables.api.spec.model.ToggleStatus; + +public interface ToggleStatusesService { + /** + * Obtain the status of a {@link com.linkedin.openhouse.housetables.api.spec.model.UserTable}'s + * feature. + * + * @param featureId identifier of the feature + * @param databaseId identifier of the database + * @param tableId identifier of the table + * @return {@link ToggleStatus} of the requested entity. + */ + ToggleStatus getTableToggleStatus(String featureId, String databaseId, String tableId); +} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java new file mode 100644 index 00000000..0c0d1b3b --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java @@ -0,0 +1,27 @@ +package com.linkedin.openhouse.housetables.services; + +import com.linkedin.openhouse.housetables.api.spec.model.ToggleStatus; +import com.linkedin.openhouse.housetables.api.spec.model.ToggleStatusEnum; +import com.linkedin.openhouse.housetables.model.TableToggleRule; +import com.linkedin.openhouse.housetables.repository.impl.jdbc.ToggleStatusHtsJdbcRepository; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +@Component +public class ToggleStatusesServiceImpl implements ToggleStatusesService { + @Autowired ToggleStatusHtsJdbcRepository htsRepository; + + @Override + public ToggleStatus getTableToggleStatus(String featureId, String databaseId, String tableId) { + for (TableToggleRule tableToggleRule : htsRepository.findAllByFeature(featureId)) { + + // TODO: Evolve this rule engine to support wildcards + if (tableToggleRule.getTablePattern().equals(tableId) + && tableToggleRule.getDatabasePattern().equals(databaseId)) { + return ToggleStatus.builder().status(ToggleStatusEnum.ACTIVE).build(); + } + } + + return ToggleStatus.builder().status(ToggleStatusEnum.INACTIVE).build(); + } +} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/UserTablesServiceImpl.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/UserTablesServiceImpl.java index 400dbd27..7b8d97f4 100644 --- a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/UserTablesServiceImpl.java +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/UserTablesServiceImpl.java @@ -15,6 +15,7 @@ import java.util.stream.StreamSupport; import org.apache.iceberg.exceptions.CommitFailedException; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.data.util.Pair; import org.springframework.orm.ObjectOptimisticLockingFailureException; import org.springframework.stereotype.Component; @@ -68,7 +69,9 @@ public Pair putUserTable(UserTable userTable) { try { returnedDto = userTablesMapper.toUserTableDto(htsJdbcRepository.save(targetUserTableRow)); - } catch (CommitFailedException | ObjectOptimisticLockingFailureException ce) { + } catch (CommitFailedException + | ObjectOptimisticLockingFailureException + | DataIntegrityViolationException e) { throw new EntityConcurrentModificationException( String.format( "databaseId : %s, tableId : %s, version: %s %s", @@ -77,7 +80,7 @@ public Pair putUserTable(UserTable userTable) { targetUserTableRow.getVersion(), "The requested user table has been modified/created by other processes."), userTablesMapper.fromUserTableToRowKey(userTable).toString(), - ce); + e); } return Pair.of(returnedDto, existingUserTableRow.isPresent()); diff --git a/services/housetables/src/main/resources/application.properties b/services/housetables/src/main/resources/application.properties index b1e514bd..564dae89 100644 --- a/services/housetables/src/main/resources/application.properties +++ b/services/housetables/src/main/resources/application.properties @@ -6,6 +6,7 @@ springdoc.swagger-ui.path=/hts/api-docs springdoc.swagger-ui.operationsSorter=method spring.jpa.hibernate.ddl-auto=none spring.sql.init.mode=always +spring.jpa.defer-datasource-initialization=true management.endpoints.web.exposure.include=health, shutdown, prometheus, beans management.endpoint.health.enabled=true management.endpoint.shutdown.enabled=true diff --git a/services/housetables/src/main/resources/data.sql b/services/housetables/src/main/resources/data.sql new file mode 100644 index 00000000..ed77188b --- /dev/null +++ b/services/housetables/src/main/resources/data.sql @@ -0,0 +1,4 @@ +-- Initial value for feature toggle tables +-- When enabling/disabling some feature, please ensure they are checked-in and reviewed through this file + +INSERT IGNORE INTO table_toggle_rule (feature, database_pattern, table_pattern, id, creation_time_ms) VALUES ('demo', 'demodb', 'demotable', DEFAULT, DEFAULT); \ No newline at end of file diff --git a/services/housetables/src/main/resources/schema.sql b/services/housetables/src/main/resources/schema.sql index 7b7d1d45..37d559cd 100644 --- a/services/housetables/src/main/resources/schema.sql +++ b/services/housetables/src/main/resources/schema.sql @@ -28,3 +28,14 @@ CREATE TABLE IF NOT EXISTS job_row ( ETL_TS datetime(6) DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6), PRIMARY KEY (job_id) ); + +CREATE TABLE IF NOT EXISTS table_toggle_rule ( + feature VARCHAR (128) NOT NULL, + database_pattern VARCHAR (128) NOT NULL, + table_pattern VARCHAR (512) NOT NULL, + id BIGINT AUTO_INCREMENT, + creation_time_ms BIGINT , + ETL_TS DATETIME(6) DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6), + PRIMARY KEY (id), + UNIQUE (feature, database_pattern, table_pattern) + ); \ No newline at end of file diff --git a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusControllerTest.java b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusControllerTest.java new file mode 100644 index 00000000..b0b161fe --- /dev/null +++ b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusControllerTest.java @@ -0,0 +1,72 @@ +package com.linkedin.openhouse.housetables.e2e.togglerule; + +import static com.linkedin.openhouse.housetables.e2e.togglerule.ToggleStatusesTestConstants.*; +import static org.hamcrest.Matchers.*; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; + +import com.linkedin.openhouse.common.test.cluster.PropertyOverrideContextInitializer; +import com.linkedin.openhouse.housetables.repository.impl.jdbc.ToggleStatusHtsJdbcRepository; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.http.MediaType; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; + +@SpringBootTest +@ContextConfiguration(initializers = PropertyOverrideContextInitializer.class) +@AutoConfigureMockMvc +public class ToggleStatusControllerTest { + @Autowired MockMvc mvc; + + @Autowired ToggleStatusHtsJdbcRepository htsRepository; + + @Test + public void testGetTableToggleStatus() throws Exception { + mvc.perform( + MockMvcRequestBuilders.get("/hts/togglestatuses") + .param("databaseId", TEST_DB_NAME) + .param("tableId", TEST_TABLE_NAME) + .param("featureId", TEST_FEATURE_NAME) + .accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()) + .andExpect(content().contentType(MediaType.APPLICATION_JSON)) + .andExpect(jsonPath("$.entity.status", is(equalTo("ACTIVE")))); + + mvc.perform( + MockMvcRequestBuilders.get("/hts/togglestatuses") + /* Knowing these are the exact Id, instead of patterns with wildcard */ + .param("databaseId", TEST_RULE_1.getDatabasePattern()) + .param("tableId", TEST_RULE_1.getTablePattern()) + .param("featureId", TEST_RULE_1.getFeature()) + .accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()) + .andExpect(content().contentType(MediaType.APPLICATION_JSON)) + .andExpect(jsonPath("$.entity.status", is(equalTo("ACTIVE")))); + + mvc.perform( + MockMvcRequestBuilders.get("/hts/togglestatuses") + /* Knowing these are the exact Id, instead of patterns with wildcard */ + .param("databaseId", TEST_RULE_2.getDatabasePattern()) + .param("tableId", TEST_RULE_2.getTablePattern()) + .param("featureId", TEST_RULE_2.getFeature()) + .accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()) + .andExpect(content().contentType(MediaType.APPLICATION_JSON)) + .andExpect(jsonPath("$.entity.status", is(equalTo("ACTIVE")))); + + mvc.perform( + MockMvcRequestBuilders.get("/hts/togglestatuses") + .param("databaseId", TEST_DB_NAME) + .param("tableId", TEST_TABLE_NAME) + .param( + "featureId", + TEST_FEATURE_NAME + "postfix") /* something that are not activated*/ + .accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()) + .andExpect(content().contentType(MediaType.APPLICATION_JSON)) + .andExpect(jsonPath("$.entity.status", is(equalTo("INACTIVE")))); + } +} diff --git a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusRepositoryTest.java b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusRepositoryTest.java new file mode 100644 index 00000000..060c79ad --- /dev/null +++ b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusRepositoryTest.java @@ -0,0 +1,33 @@ +package com.linkedin.openhouse.housetables.e2e.togglerule; + +import static com.linkedin.openhouse.housetables.e2e.togglerule.ToggleStatusesTestConstants.*; + +import com.google.common.collect.ImmutableList; +import com.linkedin.openhouse.common.test.cluster.PropertyOverrideContextInitializer; +import com.linkedin.openhouse.housetables.model.TableToggleRule; +import com.linkedin.openhouse.housetables.repository.impl.jdbc.ToggleStatusHtsJdbcRepository; +import java.util.List; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.ContextConfiguration; + +@SpringBootTest +@ContextConfiguration(initializers = PropertyOverrideContextInitializer.class) +public class ToggleStatusRepositoryTest { + @Autowired ToggleStatusHtsJdbcRepository htsRepository; + + @Test + public void testFindAllByFeature() { + List toggleRuleList = + ImmutableList.copyOf(htsRepository.findAllByFeature(TEST_RULE_0.getFeature())); + Assertions.assertEquals(2, toggleRuleList.size()); + + // Now there should be 2 rules under dummy2 feature + htsRepository.save( + TEST_RULE_0.toBuilder().feature(TEST_RULE_2.getFeature()).databasePattern("dbnew").build()); + toggleRuleList = ImmutableList.copyOf(htsRepository.findAllByFeature(TEST_RULE_2.getFeature())); + Assertions.assertEquals(2, toggleRuleList.size()); + } +} diff --git a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusesServiceTest.java b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusesServiceTest.java new file mode 100644 index 00000000..597262c2 --- /dev/null +++ b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusesServiceTest.java @@ -0,0 +1,37 @@ +package com.linkedin.openhouse.housetables.e2e.togglerule; + +import com.linkedin.openhouse.common.test.cluster.PropertyOverrideContextInitializer; +import com.linkedin.openhouse.housetables.api.spec.model.ToggleStatusEnum; +import com.linkedin.openhouse.housetables.services.ToggleStatusesService; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.jdbc.Sql; + +@SpringBootTest +@ContextConfiguration(initializers = PropertyOverrideContextInitializer.class) +@Sql({"/schema.sql", "/data.sql"}) +public class ToggleStatusesServiceTest { + @Autowired ToggleStatusesService toggleStatusesService; + + @Test + public void testActivatedTableForDummy() { + Assertions.assertEquals( + toggleStatusesService.getTableToggleStatus("demo", "demodb", "demotable").getStatus(), + ToggleStatusEnum.ACTIVE); + Assertions.assertEquals( + toggleStatusesService.getTableToggleStatus("dummy1", "db", "tbl").getStatus(), + ToggleStatusEnum.ACTIVE); + Assertions.assertEquals( + toggleStatusesService.getTableToggleStatus("dummy1", "db", "testtbl1").getStatus(), + ToggleStatusEnum.ACTIVE); + Assertions.assertEquals( + toggleStatusesService.getTableToggleStatus("dummy2", "db", "tbl").getStatus(), + ToggleStatusEnum.ACTIVE); + Assertions.assertEquals( + toggleStatusesService.getTableToggleStatus("dummy2", "db", "testtbl1").getStatus(), + ToggleStatusEnum.INACTIVE); + } +} diff --git a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusesTestConstants.java b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusesTestConstants.java new file mode 100644 index 00000000..766b8ba1 --- /dev/null +++ b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusesTestConstants.java @@ -0,0 +1,26 @@ +package com.linkedin.openhouse.housetables.e2e.togglerule; + +import com.linkedin.openhouse.housetables.model.TableToggleRule; + +public class ToggleStatusesTestConstants { + private ToggleStatusesTestConstants() { + // Util class ctor noop + } + + // Following are single parameters that aligns with rule_0 + static final String TEST_DB_NAME = "db"; + static final String TEST_TABLE_NAME = "tbl"; + static final String TEST_FEATURE_NAME = "dummy1"; + + static final TableToggleRule TEST_RULE_0 = + TableToggleRule.builder() + .feature(TEST_FEATURE_NAME) + .creationTimeMs(System.currentTimeMillis()) + .tablePattern(TEST_TABLE_NAME) + .databasePattern(TEST_DB_NAME) + .build(); + + static final TableToggleRule TEST_RULE_1 = + TEST_RULE_0.toBuilder().tablePattern("testtbl1").build(); + static final TableToggleRule TEST_RULE_2 = TEST_RULE_0.toBuilder().feature("dummy2").build(); +} diff --git a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/usertable/HtsControllerTest.java b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/usertable/HtsControllerTest.java index fed0409e..f3673f65 100644 --- a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/usertable/HtsControllerTest.java +++ b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/usertable/HtsControllerTest.java @@ -47,7 +47,9 @@ public class HtsControllerTest { public void setup() { // TODO: Use rest API to create the table and test the find/delete user table again. // For now manually create the user table upfront. - htsRepository.save(TestHouseTableModelConstants.TEST_USER_TABLE_ROW); + UserTableRow testUserTableRow = + new TestHouseTableModelConstants.TestTuple(0).get_userTableRow(); + htsRepository.save(testUserTableRow); } @AfterEach diff --git a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/usertable/HtsRepositoryTest.java b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/usertable/HtsRepositoryTest.java index bee96e82..f236a8f5 100644 --- a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/usertable/HtsRepositoryTest.java +++ b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/usertable/HtsRepositoryTest.java @@ -9,10 +9,12 @@ import com.linkedin.openhouse.housetables.model.UserTableRow; import com.linkedin.openhouse.housetables.model.UserTableRowPrimaryKey; import com.linkedin.openhouse.housetables.repository.HtsRepository; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.orm.ObjectOptimisticLockingFailureException; import org.springframework.test.context.ContextConfiguration; @@ -22,9 +24,26 @@ public class HtsRepositoryTest { @Autowired HtsRepository htsRepository; + @AfterEach + public void tearDown() { + htsRepository.deleteAll(); + } + + @Test + public void testSaveFirstRecord() { + UserTableRow testUserTableRow = + new TestHouseTableModelConstants.TestTuple(0).get_userTableRow(); + // before insertion + Assertions.assertEquals(null, testUserTableRow.getVersion()); + // after insertion + Assertions.assertEquals(0, htsRepository.save(testUserTableRow).getVersion()); + } + @Test public void testHouseTable() { - htsRepository.save(TEST_USER_TABLE_ROW); + UserTableRow testUserTableRow = + new TestHouseTableModelConstants.TestTuple(0).get_userTableRow(); + htsRepository.save(testUserTableRow); UserTableRow actual = htsRepository .findById( @@ -34,8 +53,7 @@ public void testHouseTable() { .build()) .orElse(UserTableRow.builder().build()); - assertThat(isUserTableRowEqual(TestHouseTableModelConstants.TEST_USER_TABLE_ROW, actual)) - .isTrue(); + Assertions.assertEquals(testUserTableRow, actual); htsRepository.delete(actual); } @@ -57,13 +75,21 @@ public void testDeleteUserTable() { @Test public void testSaveUserTableWithConflict() { - Long currentVersion = htsRepository.save(TEST_USER_TABLE_ROW).getVersion(); + UserTableRow testUserTableRow = + new TestHouseTableModelConstants.TestTuple(0).get_userTableRow(); + Long currentVersion = htsRepository.save(testUserTableRow).getVersion(); + // test create the table again + Exception exception = + Assertions.assertThrows( + Exception.class, + () -> htsRepository.save(testUserTableRow.toBuilder().version(null).build())); + Assertions.assertTrue(exception instanceof DataIntegrityViolationException); // test update at wrong version - Exception exception = + exception = Assertions.assertThrows( Exception.class, - () -> htsRepository.save(TEST_USER_TABLE_ROW.toBuilder().version(100L).build())); + () -> htsRepository.save(testUserTableRow.toBuilder().version(100L).build())); Assertions.assertTrue( exception instanceof ObjectOptimisticLockingFailureException | exception instanceof EntityConcurrentModificationException); @@ -72,7 +98,7 @@ public void testSaveUserTableWithConflict() { Assertions.assertNotEquals( htsRepository .save( - TEST_USER_TABLE_ROW + testUserTableRow .toBuilder() .version(currentVersion) .metadataLocation("file:/ml2") @@ -82,7 +108,7 @@ public void testSaveUserTableWithConflict() { // test update at older version exception = - Assertions.assertThrows(Exception.class, () -> htsRepository.save(TEST_USER_TABLE_ROW)); + Assertions.assertThrows(Exception.class, () -> htsRepository.save(testUserTableRow)); Assertions.assertTrue( exception instanceof ObjectOptimisticLockingFailureException | exception instanceof EntityConcurrentModificationException); @@ -90,8 +116,4 @@ public void testSaveUserTableWithConflict() { htsRepository.deleteById( UserTableRowPrimaryKey.builder().databaseId(TEST_DB_ID).tableId(TEST_TABLE_ID).build()); } - - private Boolean isUserTableRowEqual(UserTableRow expected, UserTableRow actual) { - return expected.toBuilder().version(0L).build().equals(actual.toBuilder().version(0L).build()); - } } diff --git a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/usertable/UserTablesServiceTest.java b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/usertable/UserTablesServiceTest.java index 772fd918..2062ddd2 100644 --- a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/usertable/UserTablesServiceTest.java +++ b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/usertable/UserTablesServiceTest.java @@ -37,7 +37,9 @@ public class UserTablesServiceTest { @BeforeEach public void setup() { - htsRepository.save(TestHouseTableModelConstants.TEST_USER_TABLE_ROW); + UserTableRow testUserTableRow = + new TestHouseTableModelConstants.TestTuple(0).get_userTableRow(); + htsRepository.save(testUserTableRow); htsRepository.save(testTuple1_0.get_userTableRow()); htsRepository.save(testTuple2_0.get_userTableRow()); htsRepository.save(testTuple1_1.get_userTableRow()); diff --git a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/mock/mapper/UserTableVersionMapperTest.java b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/mock/mapper/UserTableVersionMapperTest.java index b90cb0fb..c0e36325 100644 --- a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/mock/mapper/UserTableVersionMapperTest.java +++ b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/mock/mapper/UserTableVersionMapperTest.java @@ -3,6 +3,7 @@ import com.linkedin.openhouse.common.exception.EntityConcurrentModificationException; import com.linkedin.openhouse.housetables.dto.mapper.UserTableVersionMapper; import com.linkedin.openhouse.housetables.model.TestHouseTableModelConstants; +import com.linkedin.openhouse.housetables.model.UserTableRow; import java.util.Optional; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -17,30 +18,42 @@ public class UserTableVersionMapperTest { @Test void testToVersionWithNoExistingRow() { Assertions.assertEquals( - versionMapper.toVersion(TestHouseTableModelConstants.TEST_USER_TABLE, Optional.empty()), - 1L); + null, + versionMapper.toVersion(TestHouseTableModelConstants.TEST_USER_TABLE, Optional.empty())); + } + + @Test + void testToVersionWithNoExistingRowAndIncorrectTableVersion() { + Assertions.assertThrows( + EntityConcurrentModificationException.class, + () -> + versionMapper.toVersion( + TestHouseTableModelConstants.TEST_USER_TABLE.toBuilder().tableVersion("v1").build(), + Optional.empty())); } @Test void testToVersionWithExistingRowAndCorrectMetadataLocation() { + UserTableRow testUserTableRow = + new TestHouseTableModelConstants.TestTuple(0).get_userTableRow(); Assertions.assertEquals( versionMapper.toVersion( TestHouseTableModelConstants.TEST_USER_TABLE .toBuilder() - .tableVersion( - TestHouseTableModelConstants.TEST_USER_TABLE_ROW.getMetadataLocation()) + .tableVersion(testUserTableRow.getMetadataLocation()) .build(), - Optional.of(TestHouseTableModelConstants.TEST_USER_TABLE_ROW)), - TestHouseTableModelConstants.TEST_USER_TABLE_ROW.getVersion()); + Optional.of(testUserTableRow)), + testUserTableRow.getVersion()); } @Test void testToVersionWithExistingRowAndIncorrectMetadataLocation() { + UserTableRow testUserTableRow = + new TestHouseTableModelConstants.TestTuple(0).get_userTableRow(); Assertions.assertThrows( EntityConcurrentModificationException.class, () -> versionMapper.toVersion( - TestHouseTableModelConstants.TEST_USER_TABLE, - Optional.of(TestHouseTableModelConstants.TEST_USER_TABLE_ROW))); + TestHouseTableModelConstants.TEST_USER_TABLE, Optional.of(testUserTableRow))); } } diff --git a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/mock/mapper/UserTablesMapperTest.java b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/mock/mapper/UserTablesMapperTest.java index c78d0b58..42103148 100644 --- a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/mock/mapper/UserTablesMapperTest.java +++ b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/mock/mapper/UserTablesMapperTest.java @@ -3,6 +3,7 @@ import com.linkedin.openhouse.housetables.dto.mapper.UserTablesMapper; import com.linkedin.openhouse.housetables.dto.model.UserTableDto; import com.linkedin.openhouse.housetables.model.TestHouseTableModelConstants; +import com.linkedin.openhouse.housetables.model.UserTableRow; import java.util.Optional; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -15,8 +16,9 @@ public class UserTablesMapperTest { @Test void toUserTableDto() { - UserTableDto dtoAfterMapping = - userTablesMapper.toUserTableDto(TestHouseTableModelConstants.TEST_USER_TABLE_ROW); + UserTableRow testUserTableRow = + new TestHouseTableModelConstants.TestTuple(0).get_userTableRow(); + UserTableDto dtoAfterMapping = userTablesMapper.toUserTableDto(testUserTableRow); // Assert objects are equal ignoring versions Assertions.assertEquals( TestHouseTableModelConstants.TEST_USER_TABLE_DTO.toBuilder().tableVersion("").build(), @@ -36,8 +38,10 @@ void toUserTable() { @Test void toUserTableRowNullStorageType() { + UserTableRow testUserTableRow = + new TestHouseTableModelConstants.TestTuple(0).get_userTableRow(); Assertions.assertEquals( - TestHouseTableModelConstants.TEST_USER_TABLE_ROW, + testUserTableRow, userTablesMapper.toUserTableRow( TestHouseTableModelConstants.TEST_USER_TABLE.toBuilder().storageType(null).build(), Optional.empty())); @@ -45,8 +49,10 @@ void toUserTableRowNullStorageType() { @Test void toUserTableRowCustomStorageType() { + UserTableRow testUserTableRow = + new TestHouseTableModelConstants.TestTuple(0).get_userTableRow(); Assertions.assertEquals( - TestHouseTableModelConstants.TEST_USER_TABLE_ROW.toBuilder().storageType("blobfs").build(), + testUserTableRow.toBuilder().storageType("blobfs").build(), userTablesMapper.toUserTableRow( TestHouseTableModelConstants.TEST_USER_TABLE.toBuilder().storageType("blobfs").build(), Optional.empty())); diff --git a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/model/TestHouseTableModelConstants.java b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/model/TestHouseTableModelConstants.java index 2373ca6a..10808a63 100644 --- a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/model/TestHouseTableModelConstants.java +++ b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/model/TestHouseTableModelConstants.java @@ -1,5 +1,6 @@ package com.linkedin.openhouse.housetables.model; +import com.linkedin.openhouse.common.api.validator.ValidatorConstants; import com.linkedin.openhouse.housetables.api.spec.model.UserTable; import com.linkedin.openhouse.housetables.dto.model.UserTableDto; import lombok.Getter; @@ -19,7 +20,6 @@ private TestHouseTableModelConstants() { public static final String TEST_DEFAULT_STORAGE_TYPE = "hdfs"; - public static final UserTableRow TEST_USER_TABLE_ROW = tuple0.get_userTableRow(); public static final UserTableDto TEST_USER_TABLE_DTO = tuple0.get_userTableDto(); public static final UserTable TEST_USER_TABLE = tuple0.get_userTable(); @@ -51,16 +51,12 @@ public TestTuple(int tbSeq) { public TestTuple(int tbSeq, int dbSeq) { this.tableId = "test_table" + tbSeq; this.databaseId = "test_db" + dbSeq; - this.ver = - LOC_TEMPLATE - .replace("$test_db", databaseId) - .replace("$test_table", tableId) - .replace("$version", "v0"); + this.ver = ValidatorConstants.INITIAL_TABLE_VERSION; this.tableLoc = LOC_TEMPLATE .replace("$test_db", databaseId) .replace("$test_table", tableId) - .replace("$version", "v1"); + .replace("$version", "v0"); this.storageType = TEST_DEFAULT_STORAGE_TYPE; this._userTable = UserTable.builder() @@ -84,7 +80,7 @@ public TestTuple(int tbSeq, int dbSeq) { UserTableRow.builder() .tableId(tableId) .databaseId(databaseId) - .version(1L) + .version(null) .metadataLocation(tableLoc) .storageType(storageType) .build(); diff --git a/services/housetables/src/test/resources/application.properties b/services/housetables/src/test/resources/application.properties new file mode 100644 index 00000000..6a14fb46 --- /dev/null +++ b/services/housetables/src/test/resources/application.properties @@ -0,0 +1,3 @@ +spring.jpa.hibernate.ddl-auto=none +spring.sql.init.mode=always +spring.jpa.defer-datasource-initialization=true \ No newline at end of file diff --git a/services/housetables/src/test/resources/data.sql b/services/housetables/src/test/resources/data.sql new file mode 100644 index 00000000..a8a319aa --- /dev/null +++ b/services/housetables/src/test/resources/data.sql @@ -0,0 +1,4 @@ +INSERT IGNORE INTO table_toggle_rule (feature, database_pattern, table_pattern, id, creation_time_ms) VALUES ('demo', 'demodb', 'demotable', DEFAULT, 987L); +INSERT IGNORE INTO table_toggle_rule (feature, database_pattern, table_pattern, id, creation_time_ms) VALUES ('dummy1', 'db', 'tbl', DEFAULT, 987L); +INSERT IGNORE INTO table_toggle_rule (feature, database_pattern, table_pattern, id, creation_time_ms) VALUES ('dummy1', 'db', 'testtbl1', DEFAULT, 987L); +INSERT IGNORE INTO table_toggle_rule (feature, database_pattern, table_pattern, id, creation_time_ms) VALUES ('dummy2', 'db', 'tbl', DEFAULT, 987L); diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/TablesSpringApplication.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/TablesSpringApplication.java index d2464518..be7557aa 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/TablesSpringApplication.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/TablesSpringApplication.java @@ -13,6 +13,7 @@ scanBasePackages = { "com.linkedin.openhouse.tables", "com.linkedin.openhouse.tables.utils", + "com.linkedin.openhouse.tables.toggle", "com.linkedin.openhouse.cluster.configs", "com.linkedin.openhouse.cluster.storage", "com.linkedin.openhouse.common.audit", diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/config/MainApplicationConfig.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/config/MainApplicationConfig.java index 89cb536e..db55fbc7 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/config/MainApplicationConfig.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/config/MainApplicationConfig.java @@ -2,7 +2,8 @@ import com.linkedin.openhouse.cluster.metrics.TagUtils; import com.linkedin.openhouse.cluster.storage.FsStorageUtils; -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; +import com.linkedin.openhouse.cluster.storage.StorageManager; +import com.linkedin.openhouse.cluster.storage.StorageType; import com.linkedin.openhouse.common.config.BaseApplicationConfig; import com.linkedin.openhouse.common.provider.HttpConnectionPoolProviderConfig; import com.linkedin.openhouse.housetables.client.api.UserTableApi; @@ -19,9 +20,9 @@ import java.util.function.Supplier; import java.util.regex.Pattern; import java.util.stream.Collectors; +import lombok.extern.slf4j.Slf4j; +import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; -import org.apache.iceberg.hadoop.HadoopFileIO; -import org.apache.iceberg.io.FileIO; import org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting; import org.springdoc.core.customizers.OpenApiCustomiser; import org.springframework.beans.factory.annotation.Autowired; @@ -34,6 +35,7 @@ /** Main Application Configuration to load cluster properties. */ @Configuration +@Slf4j public class MainApplicationConfig extends BaseApplicationConfig { public static final String APP_NAME = "tables"; private static final Pattern VERSION_PART_PATTERN = Pattern.compile("v[0-9]+"); @@ -41,23 +43,7 @@ public class MainApplicationConfig extends BaseApplicationConfig { private static final int DNS_QUERY_TIMEOUT_SECONDS = 10; - @Autowired protected FsStorageProvider fsStorageProvider; - - /** - * Provide Iceberg {@link FileIO} object based on info provided by {@link FsStorageProvider} - * - * @return Iceberg's File abstraction {@link FileIO}. - */ - @Bean - public FileIO provideIcebergFileIO() { - if ("hadoop".equals(fsStorageProvider.storageType())) { - return new HadoopFileIO(fsStorageProvider.storageClient().getConf()); - } else { - throw new UnsupportedOperationException( - String.format( - "Storage type of %s is not supported yet.", fsStorageProvider.storageType())); - } - } + @Autowired StorageManager storageManager; /** * When cluster properties are available, obtain hts base URI and inject API client @@ -98,7 +84,17 @@ MeterRegistryCustomizer provideMeterRegistry() { Consumer> provideFileSecurer() { return pathSeqSupplier -> { try { - FsStorageUtils.securePath(fsStorageProvider.storageClient(), pathSeqSupplier.get()); + // TODO: This should use high-level storage api such as Storage::secureTableObject. + if (storageManager.getDefaultStorage().getType().equals(StorageType.HDFS) + || storageManager.getDefaultStorage().getType().equals(StorageType.LOCAL)) { + FsStorageUtils.securePath( + (FileSystem) storageManager.getDefaultStorage().getClient().getNativeClient(), + pathSeqSupplier.get()); + } else { + log.warn( + "No secure path implementation for storage type: {}", + storageManager.getDefaultStorage().getType()); + } } catch (IOException ioe) { // Throwing unchecked exception and leave the handling explicitly to the caller. throw new UncheckedIOException( diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/InternalRepositoryUtils.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/InternalRepositoryUtils.java index 377fd4a3..cd6d7323 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/InternalRepositoryUtils.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/InternalRepositoryUtils.java @@ -3,8 +3,10 @@ import static com.linkedin.openhouse.internal.catalog.mapper.HouseTableSerdeUtils.getCanonicalFieldName; import com.google.common.annotations.VisibleForTesting; -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; +import com.linkedin.openhouse.cluster.storage.Storage; +import com.linkedin.openhouse.cluster.storage.StorageManager; import com.linkedin.openhouse.common.schema.IcebergSchemaHelper; +import com.linkedin.openhouse.internal.catalog.fileio.FileIOManager; import com.linkedin.openhouse.tables.dto.mapper.iceberg.PartitionSpecMapper; import com.linkedin.openhouse.tables.dto.mapper.iceberg.PoliciesSpecMapper; import com.linkedin.openhouse.tables.dto.mapper.iceberg.TableTypeMapper; @@ -17,7 +19,6 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; -import org.apache.hadoop.fs.Path; import org.apache.iceberg.Table; import org.apache.iceberg.UpdateProperties; @@ -34,8 +35,13 @@ private InternalRepositoryUtils() { } public static java.nio.file.Path constructTablePath( - FsStorageProvider fsStorageProvider, String databaseID, String tableId, String tableUUID) { - return Paths.get(fsStorageProvider.rootPath(), databaseID, tableId + "-" + tableUUID); + StorageManager storageManager, String databaseID, String tableId, String tableUUID) { + // TODO: Default storage is used here. Support for non-default storage type per table needs to + // be added. + return Paths.get( + storageManager.getDefaultStorage().getClient().getRootPrefix(), + databaseID, + tableId + "-" + tableUUID); } /** @@ -120,13 +126,13 @@ public static boolean alterPoliciesIfNeeded( @VisibleForTesting static TableDto convertToTableDto( Table table, - FsStorageProvider fsStorageProvider, + FileIOManager fileIOManager, PartitionSpecMapper partitionSpecMapper, PoliciesSpecMapper policiesMapper, TableTypeMapper tableTypeMapper) { /* Contains everything needed to populate dto */ final Map megaProps = table.properties(); - + Storage storage = fileIOManager.getStorage(table.io()); TableDto tableDto = TableDto.builder() .tableId(megaProps.get(getCanonicalFieldName("tableId"))) @@ -135,9 +141,10 @@ static TableDto convertToTableDto( .tableUri(megaProps.get(getCanonicalFieldName("tableUri"))) .tableUUID(megaProps.get(getCanonicalFieldName("tableUUID"))) .tableLocation( - fsStorageProvider - .storageClient() - .makeQualified(new Path(megaProps.get(getCanonicalFieldName("tableLocation")))) + URI.create( + storage.getClient().getEndpoint() + + megaProps.get(getCanonicalFieldName("tableLocation"))) + .normalize() .toString()) .tableVersion(megaProps.get(getCanonicalFieldName("tableVersion"))) .tableCreator(megaProps.get(getCanonicalFieldName("tableCreator"))) diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java index f5e16ce3..6f5fccaf 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java @@ -10,7 +10,7 @@ import com.google.common.collect.Maps; import com.google.gson.GsonBuilder; import com.linkedin.openhouse.cluster.configs.ClusterProperties; -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; +import com.linkedin.openhouse.cluster.storage.StorageManager; import com.linkedin.openhouse.common.api.validator.ValidatorConstants; import com.linkedin.openhouse.common.exception.InvalidSchemaEvolutionException; import com.linkedin.openhouse.common.exception.RequestValidationFailureException; @@ -18,6 +18,7 @@ import com.linkedin.openhouse.common.metrics.MetricsConstant; import com.linkedin.openhouse.common.schema.IcebergSchemaHelper; import com.linkedin.openhouse.internal.catalog.SnapshotsUtil; +import com.linkedin.openhouse.internal.catalog.fileio.FileIOManager; import com.linkedin.openhouse.tables.common.TableType; import com.linkedin.openhouse.tables.dto.mapper.TablesMapper; import com.linkedin.openhouse.tables.dto.mapper.iceberg.PartitionSpecMapper; @@ -49,7 +50,6 @@ import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.CommitFailedException; import org.apache.iceberg.exceptions.NoSuchTableException; -import org.apache.iceberg.io.FileIO; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @@ -72,9 +72,9 @@ public class OpenHouseInternalRepositoryImpl implements OpenHouseInternalReposit @Autowired TableTypeMapper tableTypeMapper; - @Autowired FsStorageProvider fsStorageProvider; + @Autowired FileIOManager fileIOManager; - @Autowired FileIO fileIO; + @Autowired StorageManager storageManager; @Autowired MeterRegistry meterRegistry; @@ -112,7 +112,7 @@ public TableDto save(TableDto tableDto) { writeSchema, partitionSpec, constructTablePath( - fsStorageProvider, + storageManager, tableDto.getDatabaseId(), tableDto.getTableId(), tableDto.getTableUUID()) @@ -153,7 +153,7 @@ public TableDto save(TableDto tableDto) { System.currentTimeMillis() - startTime); } return convertToTableDto( - table, fsStorageProvider, partitionSpecMapper, policiesMapper, tableTypeMapper); + table, fileIOManager, partitionSpecMapper, policiesMapper, tableTypeMapper); } /** @@ -181,7 +181,7 @@ protected void updateEligibilityCheck(Table existingTable, TableDto tableDto) { * Ensure existing table's tableLocation (path to metadata.json) matches user provided baseVersion * (path to metadata.json of the table where the updates are based upon) */ - private void versionCheck(Table existingTable, TableDto mergedTableDto) { + void versionCheck(Table existingTable, TableDto mergedTableDto) { String baseTableVersion = mergedTableDto.getTableVersion(); if (existingTable != null) { @@ -457,7 +457,7 @@ public Optional findById(TableDtoPrimaryKey tableDtoPrimaryKey) { } return Optional.of( convertToTableDto( - table, fsStorageProvider, partitionSpecMapper, policiesMapper, tableTypeMapper)); + table, fileIOManager, partitionSpecMapper, policiesMapper, tableTypeMapper)); } // FIXME: Likely need a cache layer to avoid expensive tableScan. @@ -487,7 +487,7 @@ public List findAllByDatabaseId(String databaseId) { .map( table -> convertToTableDto( - table, fsStorageProvider, partitionSpecMapper, policiesMapper, tableTypeMapper)) + table, fileIOManager, partitionSpecMapper, policiesMapper, tableTypeMapper)) .collect(Collectors.toList()); } @@ -519,7 +519,7 @@ public Iterable findAll() { .map( table -> convertToTableDto( - table, fsStorageProvider, partitionSpecMapper, policiesMapper, tableTypeMapper)) + table, fileIOManager, partitionSpecMapper, policiesMapper, tableTypeMapper)) .collect(Collectors.toList()); } diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/services/IcebergSnapshotsServiceImpl.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/services/IcebergSnapshotsServiceImpl.java index 45aab51f..5f738ef6 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/services/IcebergSnapshotsServiceImpl.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/services/IcebergSnapshotsServiceImpl.java @@ -45,22 +45,23 @@ public Pair putIcebergSnapshots( TableDto tableDtoToSave = tablesMapper.toTableDto( - tableDto.orElse( - TableDto.builder() - .tableId(tableId) - .databaseId(databaseId) - .clusterId(clusterId) - .tableUri( - TableUri.builder() - .tableId(tableId) - .databaseId(databaseId) - .clusterId(clusterId) - .build() - .toString()) - .tableUUID( - tableUUIDGenerator.generateUUID(icebergSnapshotRequestBody).toString()) - .tableCreator(tableCreatorUpdater) - .build()), + tableDto.orElseGet( + () -> + TableDto.builder() + .tableId(tableId) + .databaseId(databaseId) + .clusterId(clusterId) + .tableUri( + TableUri.builder() + .tableId(tableId) + .databaseId(databaseId) + .clusterId(clusterId) + .build() + .toString()) + .tableUUID( + tableUUIDGenerator.generateUUID(icebergSnapshotRequestBody).toString()) + .tableCreator(tableCreatorUpdater) + .build()), icebergSnapshotRequestBody); if (tableDto.isPresent()) { diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/services/TablesServiceImpl.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/services/TablesServiceImpl.java index 4c7e6140..9a42ab77 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/services/TablesServiceImpl.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/services/TablesServiceImpl.java @@ -106,19 +106,22 @@ public Pair putTable( // FIXME: save method redundantly issue existence check after findById is called above TableDto tableDtoToSave = tablesMapper.toTableDto( - tableDto.orElse( - TableDto.builder() - .tableUri( - TableUri.builder() - .tableId(tableId) - .databaseId(databaseId) - .clusterId(createUpdateTableRequestBody.getClusterId()) - .build() - .toString()) - .tableUUID( - tableUUIDGenerator.generateUUID(createUpdateTableRequestBody).toString()) - .tableCreator(tableCreatorUpdater) - .build()), + tableDto.orElseGet( + () -> + TableDto.builder() + .tableUri( + TableUri.builder() + .tableId(tableId) + .databaseId(databaseId) + .clusterId(createUpdateTableRequestBody.getClusterId()) + .build() + .toString()) + .tableUUID( + tableUUIDGenerator + .generateUUID(createUpdateTableRequestBody) + .toString()) + .tableCreator(tableCreatorUpdater) + .build()), createUpdateTableRequestBody); try { return Pair.of(openHouseInternalRepository.save(tableDtoToSave), !tableDto.isPresent()); diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/BaseTableFeatureToggle.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/BaseTableFeatureToggle.java index 1b960ec5..9ad3a7cb 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/BaseTableFeatureToggle.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/BaseTableFeatureToggle.java @@ -1,5 +1,6 @@ package com.linkedin.openhouse.tables.toggle; +import com.linkedin.openhouse.housetables.client.model.ToggleStatus; import com.linkedin.openhouse.tables.toggle.model.TableToggleStatus; import com.linkedin.openhouse.tables.toggle.model.ToggleStatusKey; import com.linkedin.openhouse.tables.toggle.repository.ToggleStatusesRepository; @@ -28,8 +29,12 @@ public boolean isFeatureActivated(String databaseId, String tableId, String feat .featureId(featureId) .build(); Optional toggleStatus = toggleStatusesRepository.findById(toggleStatusKey); - // TODO: Change this once HTS PR is in + return toggleStatus.isPresent() - && toggleStatus.get().getToggleStatusEnum().equalsIgnoreCase("active"); + && toggleStatus + .get() + .getToggleStatusEnum() + .toString() + .equalsIgnoreCase(ToggleStatus.StatusEnum.ACTIVE.toString()); } } diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/model/TableToggleStatus.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/model/TableToggleStatus.java index a80d5cb5..500511b0 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/model/TableToggleStatus.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/model/TableToggleStatus.java @@ -1,5 +1,6 @@ package com.linkedin.openhouse.tables.toggle.model; +import com.linkedin.openhouse.housetables.client.model.ToggleStatus; import javax.persistence.Entity; import javax.persistence.Id; import javax.persistence.IdClass; @@ -26,6 +27,5 @@ public class TableToggleStatus { @Id private String databaseId; - // TODO: Need to change it to the shared Enum class once HTS PR is in - private String toggleStatusEnum; + private ToggleStatus.StatusEnum toggleStatusEnum; } diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/BaseToggleStatusesRepository.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/BaseToggleStatusesRepository.java index 99550aca..c3a68445 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/BaseToggleStatusesRepository.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/BaseToggleStatusesRepository.java @@ -3,11 +3,13 @@ import com.linkedin.openhouse.tables.toggle.model.TableToggleStatus; import com.linkedin.openhouse.tables.toggle.model.ToggleStatusKey; import java.util.Optional; +import org.springframework.stereotype.Component; /** * THIS IS A TEMPORARY PLACEHOLDER, without this the whole springboot application is failed to start * given missing injection entity for {@link ToggleStatusesRepository}. */ +@Component public class BaseToggleStatusesRepository implements ToggleStatusesRepository { @Override public S save(S entity) { diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/utils/TableUUIDGenerator.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/utils/TableUUIDGenerator.java index e9d3d52e..00453b27 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/utils/TableUUIDGenerator.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/utils/TableUUIDGenerator.java @@ -3,7 +3,8 @@ import com.google.common.annotations.VisibleForTesting; import com.google.gson.Gson; import com.google.gson.JsonObject; -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; +import com.linkedin.openhouse.cluster.storage.StorageManager; +import com.linkedin.openhouse.cluster.storage.StorageType; import com.linkedin.openhouse.common.exception.RequestValidationFailureException; import com.linkedin.openhouse.internal.catalog.CatalogConstants; import com.linkedin.openhouse.tables.api.spec.v0.request.CreateUpdateTableRequestBody; @@ -18,6 +19,7 @@ import java.util.Optional; import java.util.UUID; import lombok.extern.slf4j.Slf4j; +import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @@ -31,8 +33,10 @@ public class TableUUIDGenerator { // TODO: r/w of tableProperties being managed in single place. private static final String OPENHOUSE_NAMESPACE = "openhouse."; + private static final String DB_RAW_KEY = "databaseId"; + private static final String TBL_RAW_KEY = "tableId"; - @Autowired FsStorageProvider fsStorageProvider; + @Autowired StorageManager storageManager; /** * Public api to generate UUID for a {@link CreateUpdateTableRequestBody} @@ -62,14 +66,37 @@ public UUID generateUUID(CreateUpdateTableRequestBody createUpdateTableRequestBo * @return UUID */ public UUID generateUUID(IcebergSnapshotsRequestBody icebergSnapshotsRequestBody) { - return extractUUIDFromSnapshotJson( - icebergSnapshotsRequestBody.getJsonSnapshots(), - icebergSnapshotsRequestBody.getCreateUpdateTableRequestBody().getDatabaseId(), - icebergSnapshotsRequestBody.getCreateUpdateTableRequestBody().getTableId()) + return extractUUIDFromRequestBody(icebergSnapshotsRequestBody) .orElseGet( () -> generateUUID(icebergSnapshotsRequestBody.getCreateUpdateTableRequestBody())); } + /** Simple helper method to obtain tableURI from requestBody. */ + private String getTableURI(IcebergSnapshotsRequestBody icebergSnapshotsRequestBody) { + return icebergSnapshotsRequestBody.getCreateUpdateTableRequestBody().getDatabaseId() + + "." + + icebergSnapshotsRequestBody.getCreateUpdateTableRequestBody().getTableId(); + } + + /** + * Extracting the value of given key from the table properties map. The main use cases are for + * tableId and databaseId where the value captured in tblproperties preserved the casing from + * creation. This casing is critical if r/w for this table occurs in a platform with different + * casing-preservation contract. + */ + private String extractFromTblPropsIfExists( + String tableURI, Map tblProps, String rawKey) { + if (tblProps == null + || !tblProps.containsKey(OPENHOUSE_NAMESPACE + rawKey) + || tblProps.get(OPENHOUSE_NAMESPACE + rawKey) == null) { + throw new RequestValidationFailureException( + String.format( + "Provided snapshot is invalid for %s since databaseId or tableId is missing in properties", + tableURI)); + } + return tblProps.get(OPENHOUSE_NAMESPACE + rawKey); + } + /** * Helper method to extract UUID from tableProperties. A CTAS command's commit() call provides * "openhouse.tableUUID", if snapshot was not provided, this property is used and its path is @@ -111,22 +138,14 @@ private void validatePathOfProvidedRequest( String tableUUIDProperty, TableType tableType) { - // Using Ids from tableProperties is to ensure casing of these Ids are properly presented as - // they were when - // initially created. Ids carried in the requestBody, if sourced from query engine, may lose - // proper casing. - String dbIdFromProps = tableProperties.get(OPENHOUSE_NAMESPACE + "databaseId"); - String tblIdFromProps = tableProperties.get(OPENHOUSE_NAMESPACE + "tableId"); - if (dbIdFromProps == null || tblIdFromProps == null) { - throw new RequestValidationFailureException( - String.format( - "Provided snapshot is invalid for %s.%s since databaseId or tableId is missing in properties", - databaseId, tableId)); - } + String dbIdFromProps = + extractFromTblPropsIfExists(databaseId + "." + tableId, tableProperties, DB_RAW_KEY); + String tblIdFromProps = + extractFromTblPropsIfExists(databaseId + "." + tableId, tableProperties, TBL_RAW_KEY); java.nio.file.Path previousPath = InternalRepositoryUtils.constructTablePath( - fsStorageProvider, dbIdFromProps, tblIdFromProps, tableUUIDProperty); + storageManager, dbIdFromProps, tblIdFromProps, tableUUIDProperty); if (TableType.REPLICA_TABLE != tableType && !doesPathExist(previousPath)) { log.error("Previous tableLocation: {} doesn't exist", previousPath); throw new RequestValidationFailureException( @@ -135,18 +154,32 @@ private void validatePathOfProvidedRequest( } /** - * Helper method to extract UUID from List.of(jsonSnapshots) + * Helper method to extract UUID from Iceberg-Snapshots' RequestBody * *

If List is null or empty returns empty Optional. If List contains a snapshot, Snapshot is * validated by evaluating its "manifest-list" key. * - * @param jsonSnapshots - * @param databaseId - * @param tableId + * @param snapshotsRequestBody a complete snapshot request-body * @return Optional.of(UUID) */ - private Optional extractUUIDFromSnapshotJson( - List jsonSnapshots, String databaseId, String tableId) { + private Optional extractUUIDFromRequestBody( + IcebergSnapshotsRequestBody snapshotsRequestBody) { + List jsonSnapshots = snapshotsRequestBody.getJsonSnapshots(); + String tableURI = + snapshotsRequestBody.getCreateUpdateTableRequestBody().getDatabaseId() + + "." + + snapshotsRequestBody.getCreateUpdateTableRequestBody().getTableId(); + String databaseId = + extractFromTblPropsIfExists( + tableURI, + snapshotsRequestBody.getCreateUpdateTableRequestBody().getTableProperties(), + DB_RAW_KEY); + String tableId = + extractFromTblPropsIfExists( + tableURI, + snapshotsRequestBody.getCreateUpdateTableRequestBody().getTableProperties(), + TBL_RAW_KEY); + String snapshotStr = Optional.ofNullable(jsonSnapshots) .filter(l -> !l.isEmpty()) @@ -158,7 +191,8 @@ private Optional extractUUIDFromSnapshotJson( } String manifestListKey = "manifest-list"; java.nio.file.Path manifestListPath; - java.nio.file.Path databaseDirPath = Paths.get(fsStorageProvider.rootPath(), databaseId); + java.nio.file.Path databaseDirPath = + Paths.get(storageManager.getDefaultStorage().getClient().getRootPrefix(), databaseId); try { manifestListPath = @@ -230,7 +264,18 @@ private UUID extractUUIDFromExistingManifestListPath( */ private boolean doesPathExist(java.nio.file.Path tableDirPath) { try { - return fsStorageProvider.storageClient().exists(new Path(tableDirPath.toString())); + // TODO: Refactor client interaction to use high-level Storage API such as + // StorageManager::doesObjectExist + if (storageManager.getDefaultStorage().getType().equals(StorageType.HDFS) + || storageManager.getDefaultStorage().getType().equals(StorageType.LOCAL)) { + FileSystem fs = + (FileSystem) storageManager.getDefaultStorage().getClient().getNativeClient(); + return fs.exists(new Path(tableDirPath.toString())); + } else { + throw new UnsupportedOperationException( + "Unsupported storage type for checking path existence: " + + storageManager.getDefaultStorage().getType()); + } } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/BaseTableFeatureToggleTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/BaseTableFeatureToggleTest.java index 17fa59de..f17e20a2 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/BaseTableFeatureToggleTest.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/BaseTableFeatureToggleTest.java @@ -1,5 +1,6 @@ package com.linkedin.openhouse.tables.e2e.h2; +import com.linkedin.openhouse.housetables.client.model.ToggleStatus; import com.linkedin.openhouse.tables.toggle.BaseTableFeatureToggle; import com.linkedin.openhouse.tables.toggle.model.TableToggleStatus; import com.linkedin.openhouse.tables.toggle.repository.ToggleStatusesRepository; @@ -21,21 +22,21 @@ public void testDummyToggle() { .featureId("dummy") .databaseId("db1") .tableId("tbl1") - .toggleStatusEnum("ACTIVE") + .toggleStatusEnum(ToggleStatus.StatusEnum.ACTIVE) .build(); TableToggleStatus rule2 = TableToggleStatus.builder() .featureId("dummy") .databaseId("db2") .tableId("tbl2") - .toggleStatusEnum("ACTIVE") + .toggleStatusEnum(ToggleStatus.StatusEnum.ACTIVE) .build(); TableToggleStatus rule3 = TableToggleStatus.builder() .featureId("random") .databaseId("db1") .tableId("tbl3") - .toggleStatusEnum("ACTIVE") + .toggleStatusEnum(ToggleStatus.StatusEnum.ACTIVE) .build(); toggleStatusesRepository.save(rule1); toggleStatusesRepository.save(rule2); diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/DatabasesControllerTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/DatabasesControllerTest.java index 64858aaf..a57afad1 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/DatabasesControllerTest.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/DatabasesControllerTest.java @@ -8,7 +8,7 @@ import static com.linkedin.openhouse.tables.model.TableModelConstants.GET_TABLE_RESPONSE_BODY_SAME_DB; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; +import com.linkedin.openhouse.cluster.storage.StorageManager; import com.linkedin.openhouse.common.test.cluster.PropertyOverrideContextInitializer; import com.linkedin.openhouse.tables.api.spec.v0.response.GetAllDatabasesResponseBody; import com.linkedin.openhouse.tables.api.spec.v0.response.GetTableResponseBody; @@ -44,7 +44,7 @@ public class DatabasesControllerTest { @Autowired MockMvc mvc; - @Autowired FsStorageProvider fsStorageProvider; + @Autowired StorageManager storageManager; private void deleteTableAndValidateResponse(GetTableResponseBody getTableResponseBody) throws Exception { @@ -62,11 +62,11 @@ private void deleteTableAndValidateResponse(GetTableResponseBody getTableRespons @Tag("cleanUp") public void testGetAllDatabases() throws Exception { RequestAndValidateHelper.createTableAndValidateResponse( - GET_TABLE_RESPONSE_BODY, mvc, fsStorageProvider); + GET_TABLE_RESPONSE_BODY, mvc, storageManager); RequestAndValidateHelper.createTableAndValidateResponse( - GET_TABLE_RESPONSE_BODY_SAME_DB, mvc, fsStorageProvider); + GET_TABLE_RESPONSE_BODY_SAME_DB, mvc, storageManager); RequestAndValidateHelper.createTableAndValidateResponse( - GET_TABLE_RESPONSE_BODY_DIFF_DB, mvc, fsStorageProvider); + GET_TABLE_RESPONSE_BODY_DIFF_DB, mvc, storageManager); mvc.perform( MockMvcRequestBuilders.get(CURRENT_MAJOR_VERSION_PREFIX + "/databases") diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTest.java index 3c7d55ee..5bf5e645 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTest.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTest.java @@ -4,7 +4,7 @@ import static com.linkedin.openhouse.tables.model.TableModelConstants.*; import static org.apache.iceberg.types.Types.NestedField.*; -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; +import com.linkedin.openhouse.cluster.storage.StorageManager; import com.linkedin.openhouse.common.exception.InvalidSchemaEvolutionException; import com.linkedin.openhouse.common.exception.RequestValidationFailureException; import com.linkedin.openhouse.common.exception.UnsupportedClientOperationException; @@ -60,7 +60,7 @@ public class RepositoryTest { @SpyBean @Autowired OpenHouseInternalRepository openHouseInternalRepository; - @Autowired FsStorageProvider fsStorageProvider; + @Autowired StorageManager storageManager; @Autowired Catalog catalog; @@ -641,7 +641,7 @@ private void verifyTable(TableDto table) { Path path = Paths.get( "file:", - fsStorageProvider.rootPath(), + storageManager.getDefaultStorage().getClient().getRootPrefix(), table.getDatabaseId(), table.getTableId() + "-" + table.getTableUUID()); Assertions.assertEquals(TABLE_DTO.getTimePartitioning(), table.getTimePartitioning()); @@ -656,7 +656,7 @@ private void verifyTable(HouseTable table) { Assertions.assertEquals(TABLE_DTO.getTableUri(), table.getTableUri()); Path path = Paths.get( - fsStorageProvider.rootPath(), + storageManager.getDefaultStorage().getClient().getRootPrefix(), table.getDatabaseId(), table.getTableId() + "-" + table.getTableUUID()); Assertions.assertTrue(table.getTableLocation().startsWith(path.toString())); diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java index 7e989619..9eb9507f 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RepositoryTestWithSettableComponents.java @@ -5,10 +5,11 @@ import static org.mockito.Mockito.*; import com.linkedin.openhouse.cluster.metrics.micrometer.MetricsReporter; -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; +import com.linkedin.openhouse.cluster.storage.StorageManager; import com.linkedin.openhouse.common.test.cluster.PropertyOverrideContextInitializer; import com.linkedin.openhouse.internal.catalog.OpenHouseInternalTableOperations; import com.linkedin.openhouse.internal.catalog.SnapshotInspector; +import com.linkedin.openhouse.internal.catalog.fileio.FileIOManager; import com.linkedin.openhouse.internal.catalog.mapper.HouseTableMapper; import com.linkedin.openhouse.internal.catalog.model.HouseTable; import com.linkedin.openhouse.internal.catalog.model.HouseTablePrimaryKey; @@ -26,6 +27,7 @@ import java.util.HashSet; import java.util.Map; import java.util.Optional; +import javax.annotation.PostConstruct; import org.apache.iceberg.BaseTable; import org.apache.iceberg.Table; import org.apache.iceberg.catalog.Catalog; @@ -52,11 +54,11 @@ public class RepositoryTestWithSettableComponents { @SpyBean @Autowired OpenHouseInternalRepository openHouseInternalRepository; - @Autowired FsStorageProvider fsStorageProvider; + @Autowired StorageManager storageManager; @Autowired Catalog catalog; - @Autowired FileIO fileIO; + @Autowired FileIOManager fileIOManager; @Autowired SnapshotInspector snapshotInspector; @@ -64,6 +66,13 @@ public class RepositoryTestWithSettableComponents { @Autowired MeterRegistry meterRegistry; + FileIO fileIO; + + @PostConstruct + public void init() { + fileIO = fileIOManager.getFileIO(storageManager.getDefaultStorage().getType()); + } + /** * mocking the behavior of HouseTableRepository to throw exception for triggering retry when * needed. diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RequestAndValidateHelper.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RequestAndValidateHelper.java index 5d0283c2..64ac655c 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RequestAndValidateHelper.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/RequestAndValidateHelper.java @@ -15,7 +15,7 @@ import com.google.gson.Gson; import com.google.gson.JsonObject; import com.jayway.jsonpath.JsonPath; -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; +import com.linkedin.openhouse.cluster.storage.StorageManager; import com.linkedin.openhouse.tables.api.spec.v0.request.CreateUpdateTableRequestBody; import com.linkedin.openhouse.tables.api.spec.v0.request.IcebergSnapshotsRequestBody; import com.linkedin.openhouse.tables.api.spec.v0.response.GetTableResponseBody; @@ -61,7 +61,7 @@ static String obtainTableLocationFromMvcResult(MvcResult mvcResult) throws Excep static void updateTableAndValidateResponse( MockMvc mvc, - FsStorageProvider fsStorageProvider, + StorageManager storageManager, GetTableResponseBody getTableResponseBody, String previousTableLocation, boolean trueUpdate) @@ -92,7 +92,8 @@ static void updateTableAndValidateResponse( ValidationUtilities.validateUUID(mvcResult, getTableResponseBody.getTableUUID()); ValidationUtilities.validateSchema(mvcResult, getTableResponseBody.getSchema()); validateWritableTableProperties(mvcResult, getTableResponseBody.getTableProperties()); - ValidationUtilities.validateLocation(mvcResult, fsStorageProvider.rootPath()); + ValidationUtilities.validateLocation( + mvcResult, storageManager.getDefaultStorage().getClient().getRootPrefix()); ValidationUtilities.validateTimestamp( mvcResult, @@ -103,12 +104,12 @@ static void updateTableAndValidateResponse( static void updateTableAndValidateResponse( MockMvc mvc, - FsStorageProvider fsStorageProvider, + StorageManager storageManager, GetTableResponseBody getTableResponseBody, String previousTableLocation) throws Exception { updateTableAndValidateResponse( - mvc, fsStorageProvider, getTableResponseBody, previousTableLocation, true); + mvc, storageManager, getTableResponseBody, previousTableLocation, true); } /** @@ -172,17 +173,15 @@ static MvcResult updateTablePropsAndValidateResponse( } static MvcResult createTableAndValidateResponse( - GetTableResponseBody getTableResponseBody, - MockMvc mockMvc, - FsStorageProvider fsStorageProvider) + GetTableResponseBody getTableResponseBody, MockMvc mockMvc, StorageManager storageManager) throws Exception { - return createTableAndValidateResponse(getTableResponseBody, mockMvc, fsStorageProvider, false); + return createTableAndValidateResponse(getTableResponseBody, mockMvc, storageManager, false); } static MvcResult createTableAndValidateResponse( GetTableResponseBody getTableResponseBody, MockMvc mockMvc, - FsStorageProvider fsStorageProvider, + StorageManager storageManager, boolean stageCreate) throws Exception { MvcResult result = @@ -222,7 +221,8 @@ static MvcResult createTableAndValidateResponse( ValidationUtilities.validateSchema(result, getTableResponseBody.getSchema()); validateWritableTableProperties(result, getTableResponseBody.getTableProperties()); ValidationUtilities.validatePolicies(result, getTableResponseBody.getPolicies()); - ValidationUtilities.validateLocation(result, fsStorageProvider.rootPath()); + ValidationUtilities.validateLocation( + result, storageManager.getDefaultStorage().getClient().getRootPrefix()); Assertions.assertTrue( (Long) JsonPath.read(result.getResponse().getContentAsString(), "$.creationTime") > 0); // Return result if validation all passed. diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/SnapshotsControllerTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/SnapshotsControllerTest.java index 601132eb..35416a92 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/SnapshotsControllerTest.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/SnapshotsControllerTest.java @@ -7,7 +7,7 @@ import com.google.common.collect.ImmutableList; import com.jayway.jsonpath.JsonPath; -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; +import com.linkedin.openhouse.cluster.storage.StorageManager; import com.linkedin.openhouse.common.test.cluster.PropertyOverrideContextInitializer; import com.linkedin.openhouse.tables.api.spec.v0.request.CreateUpdateTableRequestBody; import com.linkedin.openhouse.tables.api.spec.v0.request.IcebergSnapshotsRequestBody; @@ -34,7 +34,6 @@ import org.apache.iceberg.Transaction; import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.catalog.TableIdentifier; -import org.apache.iceberg.io.FileIO; import org.apache.iceberg.relocated.com.google.common.collect.Lists; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.extension.ExtendWith; @@ -62,9 +61,7 @@ public class SnapshotsControllerTest { @Autowired MockMvc mvc; - @Autowired FsStorageProvider fsStorageProvider; - - @Autowired FileIO fileIo; + @Autowired StorageManager storageManager; /** For now starting with a naive object feeder. */ private static Stream responseBodyFeeder() { @@ -82,11 +79,12 @@ private static Stream responseBodyFeederUpdateReservedProps() { @ParameterizedTest @MethodSource("responseBodyFeeder") public void testPutSnapshotsAppend(GetTableResponseBody getTableResponseBody) throws Exception { - String dataFilePath = fsStorageProvider.rootPath() + "/data.orc"; + String dataFilePath = + storageManager.getDefaultStorage().getClient().getRootPrefix() + "/data.orc"; MvcResult createResult = RequestAndValidateHelper.createTableAndValidateResponse( - getTableResponseBody, mvc, fsStorageProvider); + getTableResponseBody, mvc, storageManager); GetTableResponseBody getResponseBody = buildGetTableResponseBody(createResult); IcebergSnapshotsRequestBody icebergSnapshotRequestBody = preparePutSnapshotsWithAppendRequest( @@ -105,7 +103,7 @@ public void testPutSnapshotsAppendWithStagedTable(GetTableResponseBody getTableR throws Exception { MvcResult stagedResult = RequestAndValidateHelper.createTableAndValidateResponse( - getTableResponseBody, mvc, fsStorageProvider, true); + getTableResponseBody, mvc, storageManager, true); String beforeUUID = JsonPath.read(stagedResult.getResponse().getContentAsString(), "$.tableUUID"); @@ -115,6 +113,7 @@ public void testPutSnapshotsAppendWithStagedTable(GetTableResponseBody getTableR getTableResponseBody .toBuilder() .tableVersion(INITIAL_TABLE_VERSION) + .tableProperties(tablePropsHelperForResponseBody(getTableResponseBody)) .tableUUID(beforeUUID) .build())); Map snapshotRefs = @@ -127,6 +126,7 @@ public void testPutSnapshotsAppendWithStagedTable(GetTableResponseBody getTableR buildCreateUpdateTableRequestBody(getTableResponseBody) .toBuilder() .baseTableVersion(INITIAL_TABLE_VERSION) + .tableProperties(tablePropsHelperForResponseBody(getTableResponseBody)) .build()) .jsonSnapshots(jsonSnapshots) .snapshotRefs(snapshotRefs) @@ -140,11 +140,13 @@ public void testPutSnapshotsAppendWithStagedTable(GetTableResponseBody getTableR @ParameterizedTest @MethodSource("responseBodyFeeder") public void testPutSnapshotsDelete(GetTableResponseBody getTableResponseBody) throws Exception { - String dataFilePath1 = fsStorageProvider.rootPath() + "/data1.orc"; - String dataFilePath2 = fsStorageProvider.rootPath() + "/data2.orc"; + String dataFilePath1 = + storageManager.getDefaultStorage().getClient().getRootPrefix() + "/data1.orc"; + String dataFilePath2 = + storageManager.getDefaultStorage().getClient().getRootPrefix() + "/data2.orc"; MvcResult createResult = RequestAndValidateHelper.createTableAndValidateResponse( - getTableResponseBody, mvc, fsStorageProvider); + getTableResponseBody, mvc, storageManager); GetTableResponseBody getResponseBody = buildGetTableResponseBody(createResult); // append once @@ -190,11 +192,13 @@ public void testPutSnapshotsDelete(GetTableResponseBody getTableResponseBody) th @MethodSource("responseBodyFeeder") public void testPutSnapshotsAppendMultiple(GetTableResponseBody getTableResponseBody) throws Exception { - String dataFilePath1 = fsStorageProvider.rootPath() + "/data1.orc"; - String dataFilePath2 = fsStorageProvider.rootPath() + "/data2.orc"; + String dataFilePath1 = + storageManager.getDefaultStorage().getClient().getRootPrefix() + "/data1.orc"; + String dataFilePath2 = + storageManager.getDefaultStorage().getClient().getRootPrefix() + "/data2.orc"; MvcResult createResult = RequestAndValidateHelper.createTableAndValidateResponse( - getTableResponseBody, mvc, fsStorageProvider); + getTableResponseBody, mvc, storageManager); GetTableResponseBody getResponseBody = buildGetTableResponseBody(createResult); // get old and new snapshots @@ -228,8 +232,10 @@ public void testPutSnapshotsAppendMultiple(GetTableResponseBody getTableResponse @MethodSource("responseBodyFeeder") public void testPutSnapshotsReplicaTableType(GetTableResponseBody getTableResponseBody) throws Exception { - String dataFilePath1 = fsStorageProvider.rootPath() + "/data1.orc"; - String dataFilePath2 = fsStorageProvider.rootPath() + "/data2.orc"; + String dataFilePath1 = + storageManager.getDefaultStorage().getClient().getRootPrefix() + "/data1.orc"; + String dataFilePath2 = + storageManager.getDefaultStorage().getClient().getRootPrefix() + "/data2.orc"; Map propsMap = new HashMap<>(); propsMap.put("openhouse.tableUUID", "cee3c6a3-a824-443a-832a-d4a1271e1e3e"); propsMap.put("openhouse.databaseId", getTableResponseBody.getDatabaseId()); @@ -243,7 +249,7 @@ public void testPutSnapshotsReplicaTableType(GetTableResponseBody getTableRespon .tableProperties(propsMap) .build(), mvc, - fsStorageProvider); + storageManager); GetTableResponseBody getResponseBody = buildGetTableResponseBody(createResult); // get old and new snapshots @@ -297,7 +303,7 @@ private void deleteTable(GetTableResponseBody getTableResponseBody) { @SneakyThrows private String getValidSnapshot(GetTableResponseBody getTableResponseBody) { openHouseInternalRepository.save(buildTableDto(getTableResponseBody)); - String dataPath = fsStorageProvider.rootPath() + "/data.orc"; + String dataPath = storageManager.getDefaultStorage().getClient().getRootPrefix() + "/data.orc"; DataFile dataFile = createDummyDataFile(dataPath, getPartitionSpec(getTableResponseBody)); TableIdentifier tableIdentifier = TableIdentifier.of(getTableResponseBody.getDatabaseId(), getTableResponseBody.getTableId()); @@ -327,4 +333,15 @@ private List getSnapshotsWithMultipleAppendRequests( } return snapshots; } + + /** + * For mock responseBody, ensure they are equipped with correct properties that are critical for + * casing contract. + */ + private Map tablePropsHelperForResponseBody(GetTableResponseBody responseBody) { + Map originalProps = responseBody.getTableProperties(); + originalProps.put("openhouse.databaseId", responseBody.getDatabaseId()); + originalProps.put("openhouse.tableId", responseBody.getTableId()); + return originalProps; + } } diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/SpringH2Application.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/SpringH2Application.java index cebe1aed..7b493791 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/SpringH2Application.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/SpringH2Application.java @@ -1,20 +1,14 @@ package com.linkedin.openhouse.tables.e2e.h2; -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; import com.linkedin.openhouse.common.audit.AuditHandler; import com.linkedin.openhouse.common.audit.DummyServiceAuditHandler; import com.linkedin.openhouse.common.audit.model.ServiceAuditEvent; import com.linkedin.openhouse.tables.audit.DummyTableAuditHandler; import com.linkedin.openhouse.tables.audit.model.TableAuditEvent; -import java.io.IOException; -import java.io.UncheckedIOException; import java.util.function.Consumer; import java.util.function.Supplier; import org.apache.hadoop.fs.Path; -import org.apache.hadoop.fs.permission.FsAction; -import org.apache.hadoop.fs.permission.FsPermission; import org.mockito.Mockito; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.SpringApplication; import org.springframework.boot.actuate.autoconfigure.security.servlet.ManagementWebSecurityAutoConfiguration; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; @@ -53,7 +47,6 @@ @EnableAutoConfiguration( exclude = {SecurityAutoConfiguration.class, ManagementWebSecurityAutoConfiguration.class}) public class SpringH2Application { - @Autowired protected FsStorageProvider fsStorageProvider; public static void main(String[] args) { SpringApplication.run(SpringH2Application.class, args); @@ -68,14 +61,7 @@ public static void main(String[] args) { @Primary Consumer> provideTestFileSecurer() { return pathSupplier -> { - try { - fsStorageProvider - .storageClient() - .setPermission( - pathSupplier.get(), new FsPermission(FsAction.ALL, FsAction.NONE, FsAction.NONE)); - } catch (IOException ioe) { - throw new UncheckedIOException(ioe); - } + // This is a no-op Consumer. It does nothing with the supplied Path. }; } diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/TablesControllerTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/TablesControllerTest.java index ef313f0e..09c342c3 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/TablesControllerTest.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/TablesControllerTest.java @@ -14,7 +14,7 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; import com.jayway.jsonpath.JsonPath; -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; +import com.linkedin.openhouse.cluster.storage.StorageManager; import com.linkedin.openhouse.common.audit.AuditHandler; import com.linkedin.openhouse.common.audit.model.ServiceAuditEvent; import com.linkedin.openhouse.common.test.cluster.PropertyOverrideContextInitializer; @@ -76,7 +76,7 @@ public class TablesControllerTest { @Autowired MockMvc mvc; - @Autowired FsStorageProvider fsStorageProvider; + @Autowired StorageManager storageManager; @Captor private ArgumentCaptor argCaptorServiceAudit; @@ -92,13 +92,13 @@ public void testCrudTables() throws Exception { // Create tables MvcResult mvcResultT1d1 = RequestAndValidateHelper.createTableAndValidateResponse( - GET_TABLE_RESPONSE_BODY, mvc, fsStorageProvider); + GET_TABLE_RESPONSE_BODY, mvc, storageManager); MvcResult mvcResultT2d1 = RequestAndValidateHelper.createTableAndValidateResponse( - GET_TABLE_RESPONSE_BODY_SAME_DB, mvc, fsStorageProvider); + GET_TABLE_RESPONSE_BODY_SAME_DB, mvc, storageManager); MvcResult mvcResultT1d2 = RequestAndValidateHelper.createTableAndValidateResponse( - GET_TABLE_RESPONSE_BODY_DIFF_DB, mvc, fsStorageProvider); + GET_TABLE_RESPONSE_BODY_DIFF_DB, mvc, storageManager); String tableLocation = RequestAndValidateHelper.obtainTableLocationFromMvcResult(mvcResultT1d1); String tableSameDbLocation = @@ -110,19 +110,19 @@ public void testCrudTables() throws Exception { // 200. RequestAndValidateHelper.updateTableAndValidateResponse( mvc, - fsStorageProvider, + storageManager, buildGetTableResponseBody(mvcResultT1d1), INITIAL_TABLE_VERSION, false); RequestAndValidateHelper.updateTableAndValidateResponse( mvc, - fsStorageProvider, + storageManager, buildGetTableResponseBody(mvcResultT2d1), INITIAL_TABLE_VERSION, false); RequestAndValidateHelper.updateTableAndValidateResponse( mvc, - fsStorageProvider, + storageManager, buildGetTableResponseBody(mvcResultT1d2), INITIAL_TABLE_VERSION, false); @@ -130,11 +130,11 @@ public void testCrudTables() throws Exception { // Sending the object with updated schema, expecting version moving ahead. // Creating a container GetTableResponseBody to update schema ONLY RequestAndValidateHelper.updateTableAndValidateResponse( - mvc, fsStorageProvider, evolveDummySchema(mvcResultT1d1), tableLocation); + mvc, storageManager, evolveDummySchema(mvcResultT1d1), tableLocation); RequestAndValidateHelper.updateTableAndValidateResponse( - mvc, fsStorageProvider, evolveDummySchema(mvcResultT2d1), tableSameDbLocation); + mvc, storageManager, evolveDummySchema(mvcResultT2d1), tableSameDbLocation); RequestAndValidateHelper.updateTableAndValidateResponse( - mvc, fsStorageProvider, evolveDummySchema(mvcResultT1d2), tableDiffDbLocation); + mvc, storageManager, evolveDummySchema(mvcResultT1d2), tableDiffDbLocation); RequestAndValidateHelper.listAllAndValidateResponse( mvc, @@ -154,7 +154,7 @@ public void testCrudTables() throws Exception { public void testUpdateProperties() throws Exception { MvcResult mvcResult = RequestAndValidateHelper.createTableAndValidateResponse( - GET_TABLE_RESPONSE_BODY, mvc, fsStorageProvider); + GET_TABLE_RESPONSE_BODY, mvc, storageManager); Map baseTblProps = new HashMap<>(); baseTblProps.putAll(TABLE_PROPS); @@ -258,7 +258,7 @@ public void testEmptyDatabase() throws Exception { @Test public void testCreateTableAlreadyExists() throws Exception { RequestAndValidateHelper.createTableAndValidateResponse( - GET_TABLE_RESPONSE_BODY, mvc, fsStorageProvider); + GET_TABLE_RESPONSE_BODY, mvc, storageManager); mvc.perform( MockMvcRequestBuilders.post( @@ -308,12 +308,12 @@ public void testCrudTablesWithPartitioning() throws Exception { MvcResult previousMvcResult = RequestAndValidateHelper.createTableAndValidateResponse( - getTableResponseBodyWithPartitioning, mvc, fsStorageProvider); + getTableResponseBodyWithPartitioning, mvc, storageManager); GetTableResponseBody getTableResponseBody = GetTableResponseBody.builder().build(); RequestAndValidateHelper.updateTableAndValidateResponse( mvc, - fsStorageProvider, + storageManager, // There's no actual updates for this check and it is just updating the partitioning fields buildGetTableResponseBody(previousMvcResult, getTableResponseBody), INITIAL_TABLE_VERSION, @@ -462,7 +462,7 @@ public void testUpdatePolicies() throws Exception { .policies(TABLE_POLICIES_COMPLEX) .build(), mvc, - fsStorageProvider); + storageManager); LinkedHashMap currentPolicies = JsonPath.read(mvcResult.getResponse().getContentAsString(), "$.policies"); @@ -637,13 +637,13 @@ public void testCreateTableWithTableType() throws Exception { // Create tables with tableType set in CreateUpdateTableRequest MvcResult mvcResult = RequestAndValidateHelper.createTableAndValidateResponse( - GET_TABLE_RESPONSE_BODY_WITH_TABLE_TYPE, mvc, fsStorageProvider); + GET_TABLE_RESPONSE_BODY_WITH_TABLE_TYPE, mvc, storageManager); String tableType = JsonPath.read(mvcResult.getResponse().getContentAsString(), "$.tableType"); // Sending the same object for update should expect no new object returned and status code being // 200. RequestAndValidateHelper.updateTableAndValidateResponse( - mvc, fsStorageProvider, buildGetTableResponseBody(mvcResult), INITIAL_TABLE_VERSION, false); + mvc, storageManager, buildGetTableResponseBody(mvcResult), INITIAL_TABLE_VERSION, false); Assertions.assertEquals( tableType, GET_TABLE_RESPONSE_BODY_WITH_TABLE_TYPE.getTableType().toString()); RequestAndValidateHelper.deleteTableAndValidateResponse(mvc, GET_TABLE_RESPONSE_BODY); @@ -686,11 +686,11 @@ public void testCreateTableWithIncorrectTableTypeThrowsException() throws Except @Test public void testGetAllDatabases() throws Exception { RequestAndValidateHelper.createTableAndValidateResponse( - GET_TABLE_RESPONSE_BODY, mvc, fsStorageProvider); + GET_TABLE_RESPONSE_BODY, mvc, storageManager); RequestAndValidateHelper.createTableAndValidateResponse( - GET_TABLE_RESPONSE_BODY_SAME_DB, mvc, fsStorageProvider); + GET_TABLE_RESPONSE_BODY_SAME_DB, mvc, storageManager); RequestAndValidateHelper.createTableAndValidateResponse( - GET_TABLE_RESPONSE_BODY_DIFF_DB, mvc, fsStorageProvider); + GET_TABLE_RESPONSE_BODY_DIFF_DB, mvc, storageManager); mvc.perform( MockMvcRequestBuilders.get( @@ -736,7 +736,7 @@ public void testGetAllDatabasesEmptyResult() throws Exception { @Test public void testStagedCreateDoesntExistInConsecutiveCalls() { RequestAndValidateHelper.createTableAndValidateResponse( - GET_TABLE_RESPONSE_BODY, mvc, fsStorageProvider, true); + GET_TABLE_RESPONSE_BODY, mvc, storageManager, true); // Staged table should not exist mvc.perform( MockMvcRequestBuilders.get( @@ -752,7 +752,7 @@ public void testStagedCreateDoesntExistInConsecutiveCalls() { @Test public void testServiceAuditGetTableSucceed() throws Exception { RequestAndValidateHelper.createTableAndValidateResponse( - GET_TABLE_RESPONSE_BODY, mvc, fsStorageProvider); + GET_TABLE_RESPONSE_BODY, mvc, storageManager); mvc.perform( MockMvcRequestBuilders.get(CURRENT_MAJOR_VERSION_PREFIX + "/databases/d1/tables/t1") .accept(MediaType.APPLICATION_JSON)); @@ -768,7 +768,7 @@ public void testServiceAuditGetTableSucceed() throws Exception { @Test public void testTableAuditSucceed() throws Exception { RequestAndValidateHelper.createTableAndValidateResponse( - GET_TABLE_RESPONSE_BODY, mvc, fsStorageProvider); + GET_TABLE_RESPONSE_BODY, mvc, storageManager); Mockito.verify(tableAuditHandler, atLeastOnce()).audit(argCaptorTableAudit.capture()); TableAuditEvent actualEvent = argCaptorTableAudit.getValue(); assertTrue( @@ -782,9 +782,9 @@ public void testTableAuditSucceed() throws Exception { @Test public void testSearchTablesWithDatabaseId() throws Exception { RequestAndValidateHelper.createTableAndValidateResponse( - GET_TABLE_RESPONSE_BODY, mvc, fsStorageProvider); + GET_TABLE_RESPONSE_BODY, mvc, storageManager); RequestAndValidateHelper.createTableAndValidateResponse( - GET_TABLE_RESPONSE_BODY_SAME_DB, mvc, fsStorageProvider); + GET_TABLE_RESPONSE_BODY_SAME_DB, mvc, storageManager); mvc.perform( MockMvcRequestBuilders.post( @@ -816,7 +816,7 @@ public void testSearchTablesWithDatabaseId() throws Exception { public void testUpdateSucceedsForColumnTags() throws Exception { MvcResult mvcResult = RequestAndValidateHelper.createTableAndValidateResponse( - GET_TABLE_RESPONSE_BODY, mvc, fsStorageProvider); + GET_TABLE_RESPONSE_BODY, mvc, storageManager); LinkedHashMap currentPolicies = JsonPath.read(mvcResult.getResponse().getContentAsString(), "$.policies"); diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/TablesServiceTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/TablesServiceTest.java index 5b70828e..99e90cb5 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/TablesServiceTest.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/TablesServiceTest.java @@ -6,7 +6,7 @@ import static org.assertj.core.api.Assertions.*; import com.google.common.collect.ImmutableMap; -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; +import com.linkedin.openhouse.cluster.storage.StorageManager; import com.linkedin.openhouse.common.exception.AlreadyExistsException; import com.linkedin.openhouse.common.exception.InvalidSchemaEvolutionException; import com.linkedin.openhouse.common.exception.NoSuchUserTableException; @@ -53,7 +53,7 @@ public class TablesServiceTest { @Autowired OpenHouseInternalRepository openHouseInternalRepository; - @Autowired FsStorageProvider fsStorageProvider; + @Autowired StorageManager storageManager; @MockBean AuthorizationHandler authorizationHandler; @@ -79,7 +79,7 @@ private void compareTables(TableDto expected, TableDto actual, TableDto previous Path expectedPath = Paths.get( "file:", - fsStorageProvider.rootPath(), + storageManager.getDefaultStorage().getClient().getRootPrefix(), actual.getDatabaseId(), actual.getTableId() + "-" + actual.getTableUUID()); Assertions.assertTrue(actual.getTableLocation().startsWith(expectedPath.toString())); diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/properties/CustomClusterPropertiesTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/properties/CustomClusterPropertiesTest.java index 7dbfb206..e2e15ab4 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/properties/CustomClusterPropertiesTest.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/properties/CustomClusterPropertiesTest.java @@ -1,11 +1,13 @@ package com.linkedin.openhouse.tables.mock.properties; import com.linkedin.openhouse.cluster.configs.ClusterProperties; +import com.linkedin.openhouse.cluster.storage.StorageManager; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.test.context.ContextConfiguration; @SpringBootTest @@ -14,6 +16,12 @@ public class CustomClusterPropertiesTest { @Autowired private ClusterProperties clusterProperties; + /** + * StorageManager validates storage properties, the 'cluster-test-properties.yaml' contains + * invalid storage type called "objectstore" for testing. + */ + @MockBean private StorageManager storageManager; + @Test public void testClusterProperties() { Assertions.assertEquals("TestCluster", clusterProperties.getClusterName()); diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/StorageManagerTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/StorageManagerTest.java new file mode 100644 index 00000000..d25c8db1 --- /dev/null +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/StorageManagerTest.java @@ -0,0 +1,81 @@ +package com.linkedin.openhouse.tables.mock.storage; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.when; + +import com.google.common.collect.ImmutableMap; +import com.linkedin.openhouse.cluster.storage.StorageManager; +import com.linkedin.openhouse.cluster.storage.StorageType; +import com.linkedin.openhouse.cluster.storage.configs.StorageProperties; +import java.util.Collections; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; + +@SpringBootTest +public class StorageManagerTest { + + @Autowired private StorageManager storageManager; + + @MockBean private StorageProperties storageProperties; + + @Test + public void validatePropertiesShouldThrowExceptionWhenDefaultTypeIsNullAndTypesIsNotEmpty() { + when(storageProperties.getDefaultType()).thenReturn(null); + when(storageProperties.getTypes()) + .thenReturn( + ImmutableMap.of( + StorageType.HDFS.getValue(), new StorageProperties.StorageTypeProperties())); + + assertThrows(IllegalArgumentException.class, () -> storageManager.validateProperties()); + } + + @Test + public void validatePropertiesShouldThrowExceptionWhenDefaultTypeIsNotNullAndTypesIsEmpty() { + when(storageProperties.getDefaultType()).thenReturn(StorageType.HDFS.getValue()); + when(storageProperties.getTypes()).thenReturn(Collections.emptyMap()); + + assertThrows(IllegalArgumentException.class, () -> storageManager.validateProperties()); + } + + @Test + public void validatePropertiesShouldThrowExceptionWhenDefaultTypeIsNotNullAndTypeIsNotPresent() { + when(storageProperties.getDefaultType()).thenReturn(StorageType.HDFS.getValue()); + when(storageProperties.getTypes()) + .thenReturn(ImmutableMap.of("valid", new StorageProperties.StorageTypeProperties())); + + assertThrows(IllegalArgumentException.class, () -> storageManager.validateProperties()); + } + + @Test + public void validatePropertiesShouldThrowExceptionWhenDefaultTypeIsInvalid() { + when(storageProperties.getDefaultType()).thenReturn("invalid"); + when(storageProperties.getTypes()) + .thenReturn(ImmutableMap.of("invalid", new StorageProperties.StorageTypeProperties())); + + assertThrows(IllegalArgumentException.class, () -> storageManager.validateProperties()); + } + + @Test + public void validateEmptyPropertiesGivesLocalStorage() { + when(storageProperties.getDefaultType()).thenReturn(null); + when(storageProperties.getTypes()).thenReturn(null); + + assertDoesNotThrow(() -> storageManager.validateProperties()); + assert (storageManager.getDefaultStorage().getType().equals(StorageType.LOCAL)); + assert (storageManager.getStorage(StorageType.LOCAL).isConfigured()); + } + + @Test + public void validatePropertiesGivesHdfsStorage() { + when(storageProperties.getDefaultType()).thenReturn(StorageType.HDFS.getValue()); + when(storageProperties.getTypes()) + .thenReturn( + ImmutableMap.of( + StorageType.HDFS.getValue(), new StorageProperties.StorageTypeProperties())); + + assertDoesNotThrow(() -> storageManager.validateProperties()); + } +} diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/StoragePropertiesConfigTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/StoragePropertiesConfigTest.java new file mode 100644 index 00000000..c8956100 --- /dev/null +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/StoragePropertiesConfigTest.java @@ -0,0 +1,62 @@ +package com.linkedin.openhouse.tables.mock.storage; + +import com.linkedin.openhouse.cluster.storage.StorageManager; +import com.linkedin.openhouse.cluster.storage.configs.StorageProperties; +import com.linkedin.openhouse.tables.mock.properties.CustomClusterPropertiesInitializer; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.test.context.ContextConfiguration; + +@SpringBootTest +@ContextConfiguration(initializers = CustomClusterPropertiesInitializer.class) +public class StoragePropertiesConfigTest { + @Autowired private StorageProperties storageProperties; + + @MockBean private StorageManager storageManager; + + private static final String DEFAULT_TYPE = "hdfs"; + + private static final String DEFAULT_ENDPOINT = "hdfs://localhost:9000"; + + private static final String ANOTHER_TYPE = "objectstore"; + + private static final String ANOTHER_ENDPOINT = "http://localhost:9000"; + private static final String NON_EXISTING_TYPE = "non-existing-type"; + + @Test + public void testDefaultType() { + Assertions.assertEquals(DEFAULT_TYPE, storageProperties.getDefaultType()); + } + + @Test + public void testStorageTypeEndpoint() { + Assertions.assertEquals( + DEFAULT_ENDPOINT, storageProperties.getTypes().get(DEFAULT_TYPE).getEndpoint()); + } + + @Test + public void testStorageTypeLookup() { + Assertions.assertEquals( + ANOTHER_ENDPOINT, storageProperties.getTypes().get(ANOTHER_TYPE).getEndpoint()); + } + + @Test + public void testStorageTypeVariableProperties() { + Assertions.assertFalse( + storageProperties.getTypes().get(DEFAULT_TYPE).getParameters().isEmpty()); + } + + @Test + public void testUnsetPropertiesAreNull() { + Assertions.assertNull(storageProperties.getTypes().get(NON_EXISTING_TYPE)); + } + + @AfterAll + static void unsetSysProp() { + System.clearProperty("OPENHOUSE_CLUSTER_CONFIG_PATH"); + } +} diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/StorageTypeEnumTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/StorageTypeEnumTest.java new file mode 100644 index 00000000..cd86fb42 --- /dev/null +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/StorageTypeEnumTest.java @@ -0,0 +1,72 @@ +package com.linkedin.openhouse.tables.mock.storage; + +import com.linkedin.openhouse.cluster.storage.StorageType; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class StorageTypeEnumTest { + + /** + * This test demonstrates the ability to extends the StorageType class to add new storage types. + */ + public static class ExtendedStorageType extends StorageType { + public static final Type GCS = new Type("gcs"); + public static final Type S3 = new Type("s3"); + + @Override + public Type fromString(String type) { + if (type.equals(GCS.getValue())) { + return GCS; + } else if (type.equals(S3.getValue())) { + return S3; + } + return super.fromString(type); + } + } + + @Test + public void testEnumEquality() { + Assertions.assertSame(StorageType.LOCAL, StorageType.LOCAL); + Assertions.assertNotSame(StorageType.HDFS, StorageType.LOCAL); + Assertions.assertNotSame(ExtendedStorageType.GCS, StorageType.HDFS); + Assertions.assertSame(ExtendedStorageType.HDFS, StorageType.HDFS); + Assertions.assertTrue(StorageType.LOCAL.equals(ExtendedStorageType.LOCAL)); + } + + @Test + public void testValueEquality() { + Assertions.assertEquals("local", StorageType.LOCAL.getValue()); + Assertions.assertEquals("hdfs", StorageType.HDFS.getValue()); + Assertions.assertEquals("gcs", ExtendedStorageType.GCS.getValue()); + Assertions.assertEquals("s3", ExtendedStorageType.S3.getValue()); + Assertions.assertEquals(StorageType.LOCAL.getValue(), ExtendedStorageType.LOCAL.getValue()); + } + + @Test + public void testTypeFromString() { + // Allows StorageType to be extended with new types. A primary bean can be provided. + StorageType storageType = new ExtendedStorageType(); + Assertions.assertSame(StorageType.LOCAL, storageType.fromString("local")); + Assertions.assertSame(StorageType.HDFS, storageType.fromString("hdfs")); + Assertions.assertSame(ExtendedStorageType.GCS, storageType.fromString("gcs")); + Assertions.assertSame(ExtendedStorageType.S3, storageType.fromString("s3")); + Assertions.assertSame(StorageType.LOCAL, storageType.fromString("local")); + Assertions.assertSame(StorageType.HDFS, storageType.fromString("hdfs")); + Assertions.assertSame(ExtendedStorageType.GCS, storageType.fromString("gcs")); + Assertions.assertSame(ExtendedStorageType.S3, storageType.fromString("s3")); + } + + @Test + public void testTypeToString() { + Assertions.assertEquals(StorageType.HDFS.toString(), "StorageType.Type(hdfs)"); + Assertions.assertEquals(StorageType.LOCAL.toString(), "StorageType.Type(local)"); + } + + @Test + public void testExceptionForInvalidString() { + StorageType storageType = new StorageType(); + Assertions.assertThrows( + IllegalArgumentException.class, () -> storageType.fromString("non-existing-type")); + Assertions.assertThrows(IllegalArgumentException.class, () -> storageType.fromString(null)); + } +} diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/hdfs/HdfsDelegationTokenRefresherDisabledTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/hdfs/HdfsDelegationTokenRefresherDisabledTest.java new file mode 100644 index 00000000..effc0aff --- /dev/null +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/hdfs/HdfsDelegationTokenRefresherDisabledTest.java @@ -0,0 +1,77 @@ +package com.linkedin.openhouse.tables.mock.storage.hdfs; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.dataformat.yaml.YAMLMapper; +import com.linkedin.openhouse.cluster.storage.StorageManager; +import com.linkedin.openhouse.cluster.storage.hdfs.HdfsDelegationTokenRefresher; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import lombok.SneakyThrows; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextInitializer; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.test.context.ContextConfiguration; + +@SpringBootTest +@ContextConfiguration( + initializers = + HdfsDelegationTokenRefresherDisabledTest.DisabledTokenRefreshPropertiesInitializer.class) +public class HdfsDelegationTokenRefresherDisabledTest { + + @MockBean StorageManager storageManager; + + @Autowired private ApplicationContext ctx; + + /** + * cluster-test-properties.yaml contains the following properties: + * + * 

+   *    token.refresh.enabled: "false"
+   *
+ */ + @Test + public void testRefreshIsNotEnabled() { + Assertions.assertThrows( + NoSuchBeanDefinitionException.class, () -> ctx.getBean(HdfsDelegationTokenRefresher.class)); + } + + public static class DisabledTokenRefreshPropertiesInitializer + implements ApplicationContextInitializer { + @Override + public void initialize(ConfigurableApplicationContext applicationContext) { + try { + Path tempFile = Files.createTempFile(null, ".yaml"); + String yamlContent = getYamlContent(); + Files.write(tempFile, yamlContent.getBytes()); + System.setProperty("OPENHOUSE_CLUSTER_CONFIG_PATH", tempFile.toString()); + } catch (IOException e) { + e.printStackTrace(); + } + } + + @SneakyThrows + private String getYamlContent() { + YAMLMapper yamlMapper = new YAMLMapper(); + JsonNode jsonNode = + yamlMapper.readTree( + "cluster:\n" + + " storages:\n" + + " default-type: \"hdfs\"\n" + + " types:\n" + + " hdfs:\n" + + " rootpath: \"/tmp/unittest\"\n" + + " endpoint: \"hdfs://localhost:9000\"\n" + + " parameters:\n" + + " token.refresh.enabled: \"false\"\n" + + " token.refresh.schedule.cron: \"* * * * * ?\""); + return yamlMapper.writeValueAsString(jsonNode); + } + } +} diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/hdfs/HdfsDelegationTokenRefresherEnabledTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/hdfs/HdfsDelegationTokenRefresherEnabledTest.java new file mode 100644 index 00000000..6e3462ce --- /dev/null +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/hdfs/HdfsDelegationTokenRefresherEnabledTest.java @@ -0,0 +1,99 @@ +package com.linkedin.openhouse.tables.mock.storage.hdfs; + +import static org.mockito.Mockito.*; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.dataformat.yaml.YAMLMapper; +import com.linkedin.openhouse.cluster.storage.StorageManager; +import com.linkedin.openhouse.cluster.storage.hdfs.HdfsDelegationTokenRefresher; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import lombok.SneakyThrows; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.mockito.stubbing.Answer; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.boot.test.mock.mockito.SpyBean; +import org.springframework.context.ApplicationContextInitializer; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.test.context.ContextConfiguration; + +@SpringBootTest +@ContextConfiguration( + initializers = + HdfsDelegationTokenRefresherEnabledTest.EnabledTokenRefreshPropertiesInitializer.class) +public class HdfsDelegationTokenRefresherEnabledTest { + + @MockBean StorageManager storageManager; + + @SpyBean private HdfsDelegationTokenRefresher hdfsDelegationTokenRefresher; + + /** + * cluster-test-properties.yaml contains the following properties: + * + * 
+   *    token.refresh.enabled: "true"
+   *    token.refresh.schedule.cron: "* * * * * ?"   // every second
+   *
+ */ + @Test + public void testRefresh() throws InterruptedException { + int arbitraryCountToAssertMultipleInvocations = 2; + + CountDownLatch latch = new CountDownLatch(arbitraryCountToAssertMultipleInvocations); + + doAnswer( + (Answer) + invocation -> { + latch.countDown(); + return null; + }) + .when(hdfsDelegationTokenRefresher) + .refresh(); + + if (!latch.await(5, TimeUnit.SECONDS)) { + // timer had to wait for 5 seconds and still the latch was not counted down to 0 + Assertions.fail( + String.format( + "The refresh method was not called at least %d times within 5 seconds", + arbitraryCountToAssertMultipleInvocations)); + } + } + + public static class EnabledTokenRefreshPropertiesInitializer + implements ApplicationContextInitializer { + @Override + public void initialize(ConfigurableApplicationContext applicationContext) { + try { + Path tempFile = Files.createTempFile(null, ".yaml"); + String yamlContent = getYamlContent(); + Files.write(tempFile, yamlContent.getBytes()); + System.setProperty("OPENHOUSE_CLUSTER_CONFIG_PATH", tempFile.toString()); + } catch (IOException e) { + e.printStackTrace(); + } + } + + @SneakyThrows + private String getYamlContent() { + YAMLMapper yamlMapper = new YAMLMapper(); + JsonNode jsonNode = + yamlMapper.readTree( + "cluster:\n" + + " storages:\n" + + " default-type: \"hdfs\"\n" + + " types:\n" + + " hdfs:\n" + + " rootpath: \"/tmp/unittest\"\n" + + " endpoint: \"hdfs://localhost:9000\"\n" + + " parameters:\n" + + " token.refresh.enabled: \"true\"\n" + + " token.refresh.schedule.cron: \"* * * * * ?\""); + return yamlMapper.writeValueAsString(jsonNode); + } + } +} diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/hdfs/HdfsStorageClientTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/hdfs/HdfsStorageClientTest.java new file mode 100644 index 00000000..0de94328 --- /dev/null +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/hdfs/HdfsStorageClientTest.java @@ -0,0 +1,85 @@ +package com.linkedin.openhouse.tables.mock.storage.hdfs; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.when; + +import com.google.common.collect.ImmutableMap; +import com.linkedin.openhouse.cluster.storage.StorageType; +import com.linkedin.openhouse.cluster.storage.configs.StorageProperties; +import com.linkedin.openhouse.cluster.storage.hdfs.HdfsStorageClient; +import java.util.HashMap; +import javax.annotation.PostConstruct; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.ApplicationContext; + +@SpringBootTest +public class HdfsStorageClientTest { + + @MockBean private StorageProperties storageProperties; + + @Autowired private ApplicationContext context; + + private HdfsStorageClient hdfsStorageClient; + + @PostConstruct + public void setupTest() { + when(storageProperties.getDefaultType()).thenReturn(StorageType.HDFS.getValue()); + when(storageProperties.getTypes()) + .thenReturn(ImmutableMap.of(StorageType.HDFS.getValue(), getStorageTypeProperties())); + hdfsStorageClient = context.getBean(HdfsStorageClient.class); + } + + @Test + public void testHdfsStorageClientInvalidPropertiesMissingRootPathAndEndpoint() { + when(storageProperties.getTypes()) + .thenReturn( + new HashMap<>( + ImmutableMap.of( + StorageType.HDFS.getValue(), new StorageProperties.StorageTypeProperties()))); + assertThrows(IllegalArgumentException.class, () -> hdfsStorageClient.init()); + } + + @Test + public void testHdfsStorageClientNullEmptyProperties() { + when(storageProperties.getTypes()).thenReturn(null); + assertThrows(IllegalArgumentException.class, () -> hdfsStorageClient.init()); + when(storageProperties.getTypes()).thenReturn(new HashMap<>()); + assertThrows(IllegalArgumentException.class, () -> hdfsStorageClient.init()); + when(storageProperties.getTypes()) + .thenReturn( + new HashMap() { + { + put(StorageType.HDFS.getValue(), null); + } + }); + assertThrows(IllegalArgumentException.class, () -> hdfsStorageClient.init()); + } + + @Test + public void testHdfsStorageClientValidProperties() { + when(storageProperties.getTypes()) + .thenReturn( + new HashMap<>( + ImmutableMap.of(StorageType.HDFS.getValue(), getStorageTypeProperties()))); + assertDoesNotThrow(() -> hdfsStorageClient.init()); + assert hdfsStorageClient.getNativeClient() != null; + assert hdfsStorageClient + .getNativeClient() + .getConf() + .get("fs.defaultFS") + .equals("hdfs://localhost:9000"); + } + + private StorageProperties.StorageTypeProperties getStorageTypeProperties() { + StorageProperties.StorageTypeProperties storageTypeProperties = + new StorageProperties.StorageTypeProperties(); + storageTypeProperties.setEndpoint("hdfs://localhost:9000"); + storageTypeProperties.setRootPath("/data/openhouse"); + storageTypeProperties.setParameters(new HashMap<>()); + return storageTypeProperties; + } +} diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/hdfs/HdfsStorageTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/hdfs/HdfsStorageTest.java new file mode 100644 index 00000000..f4c508ac --- /dev/null +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/hdfs/HdfsStorageTest.java @@ -0,0 +1,59 @@ +package com.linkedin.openhouse.tables.mock.storage.hdfs; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.when; + +import com.google.common.collect.ImmutableMap; +import com.linkedin.openhouse.cluster.storage.StorageType; +import com.linkedin.openhouse.cluster.storage.configs.StorageProperties; +import com.linkedin.openhouse.cluster.storage.hdfs.HdfsStorage; +import com.linkedin.openhouse.cluster.storage.hdfs.HdfsStorageClient; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; + +@SpringBootTest +public class HdfsStorageTest { + @Autowired private HdfsStorage hdfsStorage; + + @MockBean private StorageProperties storageProperties; + + @MockBean private HdfsStorageClient hdfsStorageClient; + + @Test + public void testHdfsStorageIsConfiguredWhenTypeIsProvided() { + when(storageProperties.getTypes()) + .thenReturn( + ImmutableMap.of( + StorageType.HDFS.getValue(), new StorageProperties.StorageTypeProperties())); + assertTrue(hdfsStorage.isConfigured()); + } + + @Test + public void testHdfsStorageTypeIsCorrect() { + assertEquals(StorageType.HDFS, hdfsStorage.getType()); + } + + @Test + public void testHdfsStorageClientIsNotNull() { + when(storageProperties.getTypes()) + .thenReturn( + ImmutableMap.of( + StorageType.HDFS.getValue(), new StorageProperties.StorageTypeProperties())); + assertNotNull(hdfsStorageClient); + } + + @Test + public void testHdfsStoragePropertiesReturned() { + Map testMap = ImmutableMap.of("k1", "v1", "k2", "v2"); + when(storageProperties.getTypes()) + .thenReturn( + ImmutableMap.of( + StorageType.HDFS.getValue(), + new StorageProperties.StorageTypeProperties( + "/data/openhouse", "hdfs://localhost:9000", testMap))); + assertEquals(testMap, hdfsStorage.getProperties()); + } +} diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/local/LocalStorageClientTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/local/LocalStorageClientTest.java new file mode 100644 index 00000000..aa36a3d7 --- /dev/null +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/local/LocalStorageClientTest.java @@ -0,0 +1,110 @@ +package com.linkedin.openhouse.tables.mock.storage.local; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.when; + +import com.linkedin.openhouse.cluster.storage.StorageType; +import com.linkedin.openhouse.cluster.storage.configs.StorageProperties; +import com.linkedin.openhouse.cluster.storage.local.LocalStorageClient; +import java.util.Collections; +import java.util.HashMap; +import javax.annotation.PostConstruct; +import org.apache.hadoop.fs.LocalFileSystem; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.ApplicationContext; + +@SpringBootTest +public class LocalStorageClientTest { + + @MockBean private StorageProperties storageProperties; + + @Autowired private ApplicationContext context; + + private LocalStorageClient localStorageClient; + + @PostConstruct + public void setupTest() { + when(storageProperties.getTypes()).thenReturn(null); + localStorageClient = context.getBean(LocalStorageClient.class); + } + + @Test + public void testLocalStorageClientInvalidPropertiesMissingRootPathAndEndpoint() { + when(storageProperties.getTypes()) + .thenReturn( + new HashMap<>( + Collections.singletonMap( + StorageType.LOCAL.getValue(), new StorageProperties.StorageTypeProperties()))); + assertThrows(IllegalArgumentException.class, () -> localStorageClient.init()); + } + + @Test + public void testLocalStorageClientNullProperties() { + when(storageProperties.getTypes()).thenReturn(null); + assertDoesNotThrow(() -> localStorageClient.init()); + } + + @Test + public void testLocalStorageClientEmptyMap() { + when(storageProperties.getTypes()).thenReturn(new HashMap<>()); + assertDoesNotThrow(() -> localStorageClient.init()); + } + + @Test + public void testLocalStorageClientValidProperties() { + when(storageProperties.getTypes()) + .thenReturn( + new HashMap<>( + Collections.singletonMap( + StorageType.LOCAL.getValue(), + new StorageProperties.StorageTypeProperties( + "/tmp2", "file://", new HashMap<>())))); + assertDoesNotThrow(() -> localStorageClient.init()); + } + + @Test + public void testLocalStorageClientInValidEndpoint() { + when(storageProperties.getTypes()) + .thenReturn( + new HashMap<>( + Collections.singletonMap( + StorageType.LOCAL.getValue(), + new StorageProperties.StorageTypeProperties( + "/tmp", "s3://", new HashMap<>())))); + assertThrows(IllegalArgumentException.class, () -> localStorageClient.init()); + } + + @Test + public void testLocalStorageClientInitialized() throws Exception { + when(storageProperties.getTypes()).thenReturn(null); + localStorageClient.init(); + Object client = localStorageClient.getNativeClient(); + assert client != null; + assert client instanceof LocalFileSystem; + } + + @Test + public void testLocalStorageCanCreateFile() throws Exception { + java.util.Random random = new java.util.Random(); + String tempFile = String.format("/tmp/testFile%s.orc", Math.abs(random.nextInt())); + when(storageProperties.getTypes()) + .thenReturn( + new HashMap<>( + Collections.singletonMap( + StorageType.LOCAL.getValue(), + new StorageProperties.StorageTypeProperties( + "/tmp", "file://", new HashMap<>())))); + localStorageClient.init(); + assert localStorageClient + .getNativeClient() + .createNewFile(new org.apache.hadoop.fs.Path(tempFile)); + assert localStorageClient.getNativeClient().exists(new org.apache.hadoop.fs.Path(tempFile)); + assert localStorageClient + .getNativeClient() + .delete(new org.apache.hadoop.fs.Path(tempFile), false); + } +} diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/local/LocalStorageTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/local/LocalStorageTest.java new file mode 100644 index 00000000..f7a0d84c --- /dev/null +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/storage/local/LocalStorageTest.java @@ -0,0 +1,86 @@ +package com.linkedin.openhouse.tables.mock.storage.local; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.when; + +import com.google.common.collect.ImmutableMap; +import com.linkedin.openhouse.cluster.storage.StorageType; +import com.linkedin.openhouse.cluster.storage.configs.StorageProperties; +import com.linkedin.openhouse.cluster.storage.local.LocalStorage; +import com.linkedin.openhouse.cluster.storage.local.LocalStorageClient; +import java.util.HashMap; +import org.apache.hadoop.fs.LocalFileSystem; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; + +@SpringBootTest +public class LocalStorageTest { + + @Autowired private LocalStorage localStorage; + + @MockBean private StorageProperties storageProperties; + + @MockBean private LocalStorageClient localStorageClient; + + private static final String DEFAULT_TYPE = "hdfs"; + + @Test + public void testLocalStorageIsConfiguredWhenDefaultTypeIsNull() { + when(storageProperties.getDefaultType()).thenReturn(null); + boolean result = localStorage.isConfigured(); + assertTrue(result); + } + + @Test + public void testLocalStorageIsConfiguredWhenTypesIsNull() { + when(storageProperties.getDefaultType()).thenReturn(DEFAULT_TYPE); + when(storageProperties.getTypes()).thenReturn(null); + boolean result = localStorage.isConfigured(); + assertTrue(result); + } + + @Test + public void testLocalStorageIsConfiguredWhenTypesIsEmpty() { + when(storageProperties.getDefaultType()).thenReturn(DEFAULT_TYPE); + when(storageProperties.getTypes()).thenReturn(new HashMap<>()); + boolean result = localStorage.isConfigured(); + assertTrue(result); + } + + @Test + public void testLocalStorageIsConfiguredWhenTypesContainsType() { + when(storageProperties.getDefaultType()).thenReturn(DEFAULT_TYPE); + when(storageProperties.getTypes()) + .thenReturn( + ImmutableMap.of( + StorageType.LOCAL.getValue(), new StorageProperties.StorageTypeProperties())); + boolean result = localStorage.isConfigured(); + assertTrue(result); + } + + @Test + public void testLocalStorageGetProperties() { + when(storageProperties.getTypes()) + .thenReturn( + ImmutableMap.of( + StorageType.LOCAL.getValue(), + new StorageProperties.StorageTypeProperties( + "rootPath", "endPoint", ImmutableMap.of("key", "value")))); + assertTrue(localStorage.getProperties().containsKey("key")); + assertTrue(localStorage.getProperties().containsValue("value")); + } + + @Test + public void testLocalStorageGetType() { + assertTrue(localStorage.getType().equals(StorageType.LOCAL)); + } + + @Test + public void testLocalStorageGetClient() { + LocalFileSystem localFileSystem = new LocalFileSystem(); + when(localStorageClient.getNativeClient()).thenReturn(localFileSystem); + assertTrue(localStorage.getClient().getNativeClient().equals(localFileSystem)); + } +} diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/uuid/TableUUIDGeneratorTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/uuid/TableUUIDGeneratorTest.java index 9c24fa78..514e46f3 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/uuid/TableUUIDGeneratorTest.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/mock/uuid/TableUUIDGeneratorTest.java @@ -6,7 +6,7 @@ import com.google.common.collect.ImmutableMap; import com.google.gson.Gson; import com.google.gson.JsonObject; -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; +import com.linkedin.openhouse.cluster.storage.StorageManager; import com.linkedin.openhouse.common.exception.RequestValidationFailureException; import com.linkedin.openhouse.internal.catalog.CatalogConstants; import com.linkedin.openhouse.tables.api.spec.v0.request.CreateUpdateTableRequestBody; @@ -15,8 +15,11 @@ import com.linkedin.openhouse.tables.repository.impl.InternalRepositoryUtils; import com.linkedin.openhouse.tables.utils.TableUUIDGenerator; import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import java.util.UUID; import lombok.SneakyThrows; +import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -26,7 +29,7 @@ @SpringBootTest public class TableUUIDGeneratorTest { - @Autowired private FsStorageProvider fsStorageProvider; + @Autowired private StorageManager storageManager; @Autowired private TableUUIDGenerator tableUUIDGenerator; @@ -62,13 +65,13 @@ public void testUUIDExtractedFromSnapshotSuccessfulPutSnapshot() { @Test public void testUUIDExtractedFromTablePropertySuccessfulPutSnapshot() { UUID expectedUUID = UUID.randomUUID(); - fsStorageProvider - .storageClient() - .create( - new Path( - InternalRepositoryUtils.constructTablePath( - fsStorageProvider, "db", "t", expectedUUID.toString()) - .toString())); + FileSystem fsClient = + (FileSystem) storageManager.getDefaultStorage().getClient().getNativeClient(); + fsClient.create( + new Path( + InternalRepositoryUtils.constructTablePath( + storageManager, "db", "t", expectedUUID.toString()) + .toString())); UUID existingUUID = tableUUIDGenerator.generateUUID( IcebergSnapshotsRequestBody.builder() @@ -96,13 +99,13 @@ public void testUUIDExtractedFromTablePropertySuccessfulPutSnapshot() { @Test public void testUUIDExtractedFromTablePropertySuccessfulCreateTable() { UUID expectedUUID = UUID.randomUUID(); - fsStorageProvider - .storageClient() - .create( - new Path( - InternalRepositoryUtils.constructTablePath( - fsStorageProvider, "db", "t", expectedUUID.toString()) - .toString())); + FileSystem fsClient = + (FileSystem) storageManager.getDefaultStorage().getClient().getNativeClient(); + fsClient.create( + new Path( + InternalRepositoryUtils.constructTablePath( + storageManager, "db", "t", expectedUUID.toString()) + .toString())); UUID existingUUID = tableUUIDGenerator.generateUUID( CreateUpdateTableRequestBody.builder() @@ -146,6 +149,7 @@ public void testUUIDFailsForInvalidSnapshot() { CreateUpdateTableRequestBody.builder() .tableId("t") .databaseId("db") + .tableProperties(generateMinimalTestProps("db", "t")) .clusterId(CLUSTER_NAME) .build()) .jsonSnapshots( @@ -189,11 +193,14 @@ public void testUUIDFailsForInvalidSnapshotShortManifestList() { CreateUpdateTableRequestBody.builder() .tableId("t") .databaseId("db") + .tableProperties(generateMinimalTestProps("db", "t")) .clusterId(CLUSTER_NAME) .build()) .jsonSnapshots( Collections.singletonList( - getIcebergSnapshot(fsStorageProvider.rootPath() + "/db"))) + getIcebergSnapshot( + storageManager.getDefaultStorage().getClient().getRootPrefix() + + "/db"))) .build())); } @@ -210,12 +217,14 @@ public void testUUIDFailsForInvalidSnapshotWithoutUUID() { CreateUpdateTableRequestBody.builder() .tableId("t") .databaseId("db") + .tableProperties(generateMinimalTestProps("db", "t")) .clusterId(CLUSTER_NAME) .build()) .jsonSnapshots( Collections.singletonList( getIcebergSnapshot( - fsStorageProvider.rootPath() + "/db/t-NOTUUID/maniffest-list"))) + storageManager.getDefaultStorage().getClient().getRootPrefix() + + "/db/t-NOTUUID/maniffest-list"))) .build())); Assertions.assertTrue(exception.getMessage().contains("contains invalid UUID")); } @@ -288,7 +297,7 @@ private String getIcebergSnapshot( String databaseId, String tableId, UUID tableUUID, String appendedPath) { return getIcebergSnapshot( InternalRepositoryUtils.constructTablePath( - fsStorageProvider, databaseId, tableId, tableUUID.toString()) + storageManager, databaseId, tableId, tableUUID.toString()) + appendedPath); } @@ -298,4 +307,13 @@ private String getIcebergSnapshot(String manifestListValue) { jsonObject.addProperty(key, manifestListValue); return jsonObject.toString(); } + + private Map generateMinimalTestProps(String databaseId, String tableId) { + return new HashMap() { + { + put("openhouse.databaseId", databaseId); + put("openhouse.tableId", tableId); + } + }; + } } diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/repository/impl/InternalRepositoryUtilsTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/repository/impl/InternalRepositoryUtilsTest.java index e05fdb71..92faba13 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/repository/impl/InternalRepositoryUtilsTest.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/repository/impl/InternalRepositoryUtilsTest.java @@ -1,215 +1,216 @@ -package com.linkedin.openhouse.tables.repository.impl; - -import static com.linkedin.openhouse.tables.model.TableModelConstants.TABLE_DTO; -import static org.mockito.Mockito.doReturn; - -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; -import com.linkedin.openhouse.tables.common.TableType; -import com.linkedin.openhouse.tables.dto.mapper.iceberg.PartitionSpecMapper; -import com.linkedin.openhouse.tables.dto.mapper.iceberg.PoliciesSpecMapper; -import com.linkedin.openhouse.tables.dto.mapper.iceberg.TableTypeMapper; -import com.linkedin.openhouse.tables.model.TableDto; -import java.io.File; -import java.lang.reflect.Field; -import java.lang.reflect.Modifier; -import java.net.URI; -import java.nio.file.Paths; -import java.util.Arrays; -import java.util.HashMap; -import java.util.Map; -import java.util.UUID; -import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.fs.FileSystem; -import org.apache.iceberg.Schema; -import org.apache.iceberg.Table; -import org.apache.iceberg.UpdateProperties; -import org.apache.iceberg.types.Types; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; -import org.mockito.Mockito; -import org.springframework.util.ReflectionUtils; - -class InternalRepositoryUtilsTest { - - @Test - void testAlterPropIfNeeded() { - UpdateProperties mockUpdateProperties = Mockito.mock(UpdateProperties.class); - doReturn(null).when(mockUpdateProperties).set(Mockito.anyString(), Mockito.anyString()); - doReturn(null).when(mockUpdateProperties).remove(Mockito.anyString()); - - Map existingTableProps = new HashMap<>(); - Map providedTableProps = new HashMap<>(); - - // Note that the sequence of following test cases matters. - - // providedTableProps:{} - // existingTableProps:{} - Assertions.assertFalse( - InternalRepositoryUtils.alterPropIfNeeded( - mockUpdateProperties, existingTableProps, providedTableProps)); - - // insert - // providedTableProps:{a:aa} - // existingTableProps:{} - providedTableProps.put("a", "aa"); - Assertions.assertTrue( - InternalRepositoryUtils.alterPropIfNeeded( - mockUpdateProperties, existingTableProps, providedTableProps)); - existingTableProps.put("a", "aa"); - - // upsert - // providedTableProps:{a:bb} - // existingTableProps:{a:aa} - providedTableProps.put("a", "bb"); - Assertions.assertTrue( - InternalRepositoryUtils.alterPropIfNeeded( - mockUpdateProperties, existingTableProps, providedTableProps)); - existingTableProps.put("a", "bb"); - - // unset - // providedTableProps:{} - // existingTableProps:{a:bb} - providedTableProps.clear(); - Assertions.assertTrue( - InternalRepositoryUtils.alterPropIfNeeded( - mockUpdateProperties, existingTableProps, providedTableProps)); - existingTableProps.remove("a"); - } - - @Test - void testRemovingHtsField() throws Exception { - Map test = new HashMap<>(); - test.put("openhouse.tableId", "a"); - test.put("openhouse.databaseId", "b"); - test.put("openhouse.clusterId", "c"); - test.put("openhouse.tableUri", "d"); - test.put("openhouse.tableUUID", "e"); - test.put("openhouse.tableLocation", "f"); - test.put("openhouse.tableVersion", "g"); - test.put("openhouse.tableType", "type"); - test.put("policies", "po"); - Map result = - InternalRepositoryUtils.getUserDefinedTblProps(test, new BasePreservedKeyCheckerTest()); - Assertions.assertTrue(result.isEmpty()); - } - - @Test - void testConvertToTableDto() throws Exception { - File tmpFile = File.createTempFile("foo", "bar"); - - final Schema SCHEMA = - new Schema( - Types.NestedField.required(1, "id", Types.LongType.get()), - Types.NestedField.required(2, "ts", Types.TimestampType.withZone())); - - Map tableProps = new HashMap<>(); - tableProps.put("openhouse.tableId", "a"); - tableProps.put("openhouse.databaseId", "b"); - tableProps.put("openhouse.clusterId", "c"); - tableProps.put("openhouse.tableUri", "d"); - tableProps.put("openhouse.tableUUID", "e"); - tableProps.put("openhouse.tableLocation", tmpFile.getAbsolutePath()); - tableProps.put("openhouse.tableVersion", "g"); - tableProps.put("openhouse.tableCreator", "f"); - tableProps.put("openhouse.lastModifiedTime", "1000"); - tableProps.put("openhouse.creationTime", "1000"); - tableProps.put("openhouse.tableType", TableType.PRIMARY_TABLE.toString()); - - Table mockTable = Mockito.mock(Table.class); - Mockito.doReturn(tableProps).when(mockTable).properties(); - Mockito.doReturn(SCHEMA).when(mockTable).schema(); - FsStorageProvider mockFsStorageProvider = Mockito.mock(FsStorageProvider.class); - Mockito.doReturn(FileSystem.get(new Configuration())) - .when(mockFsStorageProvider) - .storageClient(); - PartitionSpecMapper mockPartitionSpecMapper = Mockito.mock(PartitionSpecMapper.class); - Mockito.doReturn(null).when(mockPartitionSpecMapper).toPartitionSpec(Mockito.any()); - Mockito.doReturn(TABLE_DTO.getClustering()) - .when(mockPartitionSpecMapper) - .toClusteringSpec(Mockito.any()); - Mockito.doReturn(TABLE_DTO.getTimePartitioning()) - .when(mockPartitionSpecMapper) - .toTimePartitionSpec(Mockito.any()); - PoliciesSpecMapper mockPolicyMapper = Mockito.mock(PoliciesSpecMapper.class); - Mockito.doReturn(TABLE_DTO.getPolicies()) - .when(mockPolicyMapper) - .toPoliciesObject(Mockito.any()); - - TableTypeMapper mockTableTypeMapper = Mockito.mock(TableTypeMapper.class); - Mockito.doReturn(TableType.PRIMARY_TABLE).when(mockTableTypeMapper).toTableType(Mockito.any()); - - TableDto returnDto = - InternalRepositoryUtils.convertToTableDto( - mockTable, - mockFsStorageProvider, - mockPartitionSpecMapper, - mockPolicyMapper, - mockTableTypeMapper); - Assertions.assertEquals(returnDto.getTableId(), "a"); - Assertions.assertEquals(returnDto.getDatabaseId(), "b"); - Assertions.assertEquals(returnDto.getClusterId(), "c"); - Assertions.assertEquals(returnDto.getTableUri(), "d"); - Assertions.assertEquals(returnDto.getTableUUID(), "e"); - Assertions.assertEquals( - URI.create(returnDto.getTableLocation()).getPath(), tmpFile.getAbsolutePath()); - Assertions.assertEquals(returnDto.getTableVersion(), "g"); - Assertions.assertEquals(returnDto.getTableCreator(), "f"); - Assertions.assertEquals(returnDto.getCreationTime(), 1000); - Assertions.assertEquals(returnDto.getLastModifiedTime(), 1000); - Assertions.assertEquals(returnDto.getTableType(), TableType.valueOf("PRIMARY_TABLE")); - - // All internal fields of TableDTO has to carry non-null value except jsonSnapshots which is - // intentional. - // Point of this check is, if something is added into TableDTO but somehow left explicit - // handling in convertToTableDto - // this test shall fail. - - Arrays.stream(TableDto.class.getDeclaredFields()) - .filter( - field -> - Modifier.isPrivate(field.getModifiers()) - && !Modifier.isStatic(field.getModifiers()) - && !field.getName().equals("jsonSnapshots") - && !field.getName().equals("snapshotRefs")) - .map(Field::getName) - .forEach(name -> ensurePrivateFieldNonNull(name, returnDto)); - - tableProps.remove("openhouse.lastModifiedTime"); - tableProps.remove("openhouse.creationTime"); - TableDto returnDto2 = - InternalRepositoryUtils.convertToTableDto( - mockTable, - mockFsStorageProvider, - mockPartitionSpecMapper, - mockPolicyMapper, - mockTableTypeMapper); - Assertions.assertEquals(returnDto2.getCreationTime(), 0); - Assertions.assertEquals(returnDto2.getLastModifiedTime(), 0); - } - - @Test - void testConstructTablePath() { - FsStorageProvider fsStorageProvider = Mockito.mock(FsStorageProvider.class); - String someRoot = "root"; - String dbId = "db"; - String tbId = "tb"; - String uuid = UUID.randomUUID().toString(); - Mockito.when(fsStorageProvider.rootPath()).thenReturn(someRoot); - Assertions.assertEquals( - InternalRepositoryUtils.constructTablePath(fsStorageProvider, dbId, tbId, uuid), - Paths.get(someRoot, dbId, tbId + "-" + uuid)); - } - - private void ensurePrivateFieldNonNull(String fieldName, TableDto tableDto) { - Field resultField = ReflectionUtils.findField(TableDto.class, fieldName); - Assertions.assertNotNull(resultField); - ReflectionUtils.makeAccessible(resultField); - Assertions.assertNotNull(ReflectionUtils.getField(resultField, tableDto)); - } - - private class BasePreservedKeyCheckerTest extends BasePreservedKeyChecker { - public BasePreservedKeyCheckerTest() { - // empty constructor, purely for testing purpose. - } - } -} +// package com.linkedin.openhouse.tables.repository.impl; +// +// import static com.linkedin.openhouse.tables.model.TableModelConstants.TABLE_DTO; +// import static org.mockito.Mockito.doReturn; +// +// import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; +// import com.linkedin.openhouse.tables.common.TableType; +// import com.linkedin.openhouse.tables.dto.mapper.iceberg.PartitionSpecMapper; +// import com.linkedin.openhouse.tables.dto.mapper.iceberg.PoliciesSpecMapper; +// import com.linkedin.openhouse.tables.dto.mapper.iceberg.TableTypeMapper; +// import com.linkedin.openhouse.tables.model.TableDto; +// import java.io.File; +// import java.lang.reflect.Field; +// import java.lang.reflect.Modifier; +// import java.net.URI; +// import java.nio.file.Paths; +// import java.util.Arrays; +// import java.util.HashMap; +// import java.util.Map; +// import java.util.UUID; +// import org.apache.hadoop.conf.Configuration; +// import org.apache.hadoop.fs.FileSystem; +// import org.apache.iceberg.Schema; +// import org.apache.iceberg.Table; +// import org.apache.iceberg.UpdateProperties; +// import org.apache.iceberg.types.Types; +// import org.junit.jupiter.api.Assertions; +// import org.junit.jupiter.api.Test; +// import org.mockito.Mockito; +// import org.springframework.util.ReflectionUtils; +// +// class InternalRepositoryUtilsTest { +// +// @Test +// void testAlterPropIfNeeded() { +// UpdateProperties mockUpdateProperties = Mockito.mock(UpdateProperties.class); +// doReturn(null).when(mockUpdateProperties).set(Mockito.anyString(), Mockito.anyString()); +// doReturn(null).when(mockUpdateProperties).remove(Mockito.anyString()); +// +// Map existingTableProps = new HashMap<>(); +// Map providedTableProps = new HashMap<>(); +// +// // Note that the sequence of following test cases matters. +// +// // providedTableProps:{} +// // existingTableProps:{} +// Assertions.assertFalse( +// InternalRepositoryUtils.alterPropIfNeeded( +// mockUpdateProperties, existingTableProps, providedTableProps)); +// +// // insert +// // providedTableProps:{a:aa} +// // existingTableProps:{} +// providedTableProps.put("a", "aa"); +// Assertions.assertTrue( +// InternalRepositoryUtils.alterPropIfNeeded( +// mockUpdateProperties, existingTableProps, providedTableProps)); +// existingTableProps.put("a", "aa"); +// +// // upsert +// // providedTableProps:{a:bb} +// // existingTableProps:{a:aa} +// providedTableProps.put("a", "bb"); +// Assertions.assertTrue( +// InternalRepositoryUtils.alterPropIfNeeded( +// mockUpdateProperties, existingTableProps, providedTableProps)); +// existingTableProps.put("a", "bb"); +// +// // unset +// // providedTableProps:{} +// // existingTableProps:{a:bb} +// providedTableProps.clear(); +// Assertions.assertTrue( +// InternalRepositoryUtils.alterPropIfNeeded( +// mockUpdateProperties, existingTableProps, providedTableProps)); +// existingTableProps.remove("a"); +// } +// +// @Test +// void testRemovingHtsField() throws Exception { +// Map test = new HashMap<>(); +// test.put("openhouse.tableId", "a"); +// test.put("openhouse.databaseId", "b"); +// test.put("openhouse.clusterId", "c"); +// test.put("openhouse.tableUri", "d"); +// test.put("openhouse.tableUUID", "e"); +// test.put("openhouse.tableLocation", "f"); +// test.put("openhouse.tableVersion", "g"); +// test.put("openhouse.tableType", "type"); +// test.put("policies", "po"); +// Map result = +// InternalRepositoryUtils.getUserDefinedTblProps(test, new BasePreservedKeyCheckerTest()); +// Assertions.assertTrue(result.isEmpty()); +// } +// +// @Test +// void testConvertToTableDto() throws Exception { +// File tmpFile = File.createTempFile("foo", "bar"); +// +// final Schema SCHEMA = +// new Schema( +// Types.NestedField.required(1, "id", Types.LongType.get()), +// Types.NestedField.required(2, "ts", Types.TimestampType.withZone())); +// +// Map tableProps = new HashMap<>(); +// tableProps.put("openhouse.tableId", "a"); +// tableProps.put("openhouse.databaseId", "b"); +// tableProps.put("openhouse.clusterId", "c"); +// tableProps.put("openhouse.tableUri", "d"); +// tableProps.put("openhouse.tableUUID", "e"); +// tableProps.put("openhouse.tableLocation", tmpFile.getAbsolutePath()); +// tableProps.put("openhouse.tableVersion", "g"); +// tableProps.put("openhouse.tableCreator", "f"); +// tableProps.put("openhouse.lastModifiedTime", "1000"); +// tableProps.put("openhouse.creationTime", "1000"); +// tableProps.put("openhouse.tableType", TableType.PRIMARY_TABLE.toString()); +// +// Table mockTable = Mockito.mock(Table.class); +// Mockito.doReturn(tableProps).when(mockTable).properties(); +// Mockito.doReturn(SCHEMA).when(mockTable).schema(); +// FsStorageProvider mockFsStorageProvider = Mockito.mock(FsStorageProvider.class); +// Mockito.doReturn(FileSystem.get(new Configuration())) +// .when(mockFsStorageProvider) +// .storageClient(); +// PartitionSpecMapper mockPartitionSpecMapper = Mockito.mock(PartitionSpecMapper.class); +// Mockito.doReturn(null).when(mockPartitionSpecMapper).toPartitionSpec(Mockito.any()); +// Mockito.doReturn(TABLE_DTO.getClustering()) +// .when(mockPartitionSpecMapper) +// .toClusteringSpec(Mockito.any()); +// Mockito.doReturn(TABLE_DTO.getTimePartitioning()) +// .when(mockPartitionSpecMapper) +// .toTimePartitionSpec(Mockito.any()); +// PoliciesSpecMapper mockPolicyMapper = Mockito.mock(PoliciesSpecMapper.class); +// Mockito.doReturn(TABLE_DTO.getPolicies()) +// .when(mockPolicyMapper) +// .toPoliciesObject(Mockito.any()); +// +// TableTypeMapper mockTableTypeMapper = Mockito.mock(TableTypeMapper.class); +// +// Mockito.doReturn(TableType.PRIMARY_TABLE).when(mockTableTypeMapper).toTableType(Mockito.any()); +// +// TableDto returnDto = +// InternalRepositoryUtils.convertToTableDto( +// mockTable, +// mockFsStorageProvider, +// mockPartitionSpecMapper, +// mockPolicyMapper, +// mockTableTypeMapper); +// Assertions.assertEquals(returnDto.getTableId(), "a"); +// Assertions.assertEquals(returnDto.getDatabaseId(), "b"); +// Assertions.assertEquals(returnDto.getClusterId(), "c"); +// Assertions.assertEquals(returnDto.getTableUri(), "d"); +// Assertions.assertEquals(returnDto.getTableUUID(), "e"); +// Assertions.assertEquals( +// URI.create(returnDto.getTableLocation()).getPath(), tmpFile.getAbsolutePath()); +// Assertions.assertEquals(returnDto.getTableVersion(), "g"); +// Assertions.assertEquals(returnDto.getTableCreator(), "f"); +// Assertions.assertEquals(returnDto.getCreationTime(), 1000); +// Assertions.assertEquals(returnDto.getLastModifiedTime(), 1000); +// Assertions.assertEquals(returnDto.getTableType(), TableType.valueOf("PRIMARY_TABLE")); +// +// // All internal fields of TableDTO has to carry non-null value except jsonSnapshots which is +// // intentional. +// // Point of this check is, if something is added into TableDTO but somehow left explicit +// // handling in convertToTableDto +// // this test shall fail. +// +// Arrays.stream(TableDto.class.getDeclaredFields()) +// .filter( +// field -> +// Modifier.isPrivate(field.getModifiers()) +// && !Modifier.isStatic(field.getModifiers()) +// && !field.getName().equals("jsonSnapshots") +// && !field.getName().equals("snapshotRefs")) +// .map(Field::getName) +// .forEach(name -> ensurePrivateFieldNonNull(name, returnDto)); +// +// tableProps.remove("openhouse.lastModifiedTime"); +// tableProps.remove("openhouse.creationTime"); +// TableDto returnDto2 = +// InternalRepositoryUtils.convertToTableDto( +// mockTable, +// mockFsStorageProvider, +// mockPartitionSpecMapper, +// mockPolicyMapper, +// mockTableTypeMapper); +// Assertions.assertEquals(returnDto2.getCreationTime(), 0); +// Assertions.assertEquals(returnDto2.getLastModifiedTime(), 0); +// } +// +// @Test +// void testConstructTablePath() { +// FsStorageProvider fsStorageProvider = Mockito.mock(FsStorageProvider.class); +// String someRoot = "root"; +// String dbId = "db"; +// String tbId = "tb"; +// String uuid = UUID.randomUUID().toString(); +// Mockito.when(fsStorageProvider.rootPath()).thenReturn(someRoot); +// Assertions.assertEquals( +// InternalRepositoryUtils.constructTablePath(fsStorageProvider, dbId, tbId, uuid), +// Paths.get(someRoot, dbId, tbId + "-" + uuid)); +// } +// +// private void ensurePrivateFieldNonNull(String fieldName, TableDto tableDto) { +// Field resultField = ReflectionUtils.findField(TableDto.class, fieldName); +// Assertions.assertNotNull(resultField); +// ReflectionUtils.makeAccessible(resultField); +// Assertions.assertNotNull(ReflectionUtils.getField(resultField, tableDto)); +// } +// +// private class BasePreservedKeyCheckerTest extends BasePreservedKeyChecker { +// public BasePreservedKeyCheckerTest() { +// // empty constructor, purely for testing purpose. +// } +// } +// } diff --git a/services/tables/src/test/resources/cluster-test-properties.yaml b/services/tables/src/test/resources/cluster-test-properties.yaml index 5e5e44b6..5e193f99 100644 --- a/services/tables/src/test/resources/cluster-test-properties.yaml +++ b/services/tables/src/test/resources/cluster-test-properties.yaml @@ -2,6 +2,20 @@ cluster: name: "TestCluster" storage: root-path: "/tmp/unittest" + storages: + default-type: "hdfs" + types: + hdfs: + rootpath: "/tmp/unittest" + endpoint: "hdfs://localhost:9000" + parameters: + key1: value1 + objectstore: + rootpath: "tmpbucket" + endpoint: "http://localhost:9000" + parameters: + key2: value2 + token: xyz housetables: base-uri: "http://localhost:8080" security: diff --git a/tables-service.Dockerfile b/tables-service.Dockerfile index 6da9e528..b094940f 100644 --- a/tables-service.Dockerfile +++ b/tables-service.Dockerfile @@ -1,4 +1,4 @@ -FROM openjdk:11 +FROM openjdk:23-ea-11-slim ARG USER=openhouse diff --git a/tables-test-fixtures/build.gradle b/tables-test-fixtures/build.gradle index a52a2c89..04ee17f6 100644 --- a/tables-test-fixtures/build.gradle +++ b/tables-test-fixtures/build.gradle @@ -42,6 +42,7 @@ dependencies { shadowJar { zip64 = true // Required for Spring + archiveClassifier.set('uber') mergeServiceFiles() append 'META-INF/spring.handlers' append 'META-INF/spring.schemas' diff --git a/tables-test-fixtures/src/main/java/com/linkedin/openhouse/tablestest/HouseTablesH2Repository.java b/tables-test-fixtures/src/main/java/com/linkedin/openhouse/tablestest/HouseTablesH2Repository.java index 49a718ad..5228fa7c 100644 --- a/tables-test-fixtures/src/main/java/com/linkedin/openhouse/tablestest/HouseTablesH2Repository.java +++ b/tables-test-fixtures/src/main/java/com/linkedin/openhouse/tablestest/HouseTablesH2Repository.java @@ -1,6 +1,9 @@ package com.linkedin.openhouse.tablestest; +import com.linkedin.openhouse.internal.catalog.model.HouseTable; +import com.linkedin.openhouse.internal.catalog.model.HouseTablePrimaryKey; import com.linkedin.openhouse.internal.catalog.repository.HouseTableRepository; +import java.util.Optional; import org.springframework.context.annotation.Primary; import org.springframework.stereotype.Repository; @@ -11,4 +14,13 @@ */ @Repository @Primary -public interface HouseTablesH2Repository extends HouseTableRepository {} +public interface HouseTablesH2Repository extends HouseTableRepository { + Optional findByDatabaseIdIgnoreCaseAndTableIdIgnoreCase( + String databaseId, String tableId); + + @Override + default Optional findById(HouseTablePrimaryKey houseTablePrimaryKey) { + return this.findByDatabaseIdIgnoreCaseAndTableIdIgnoreCase( + houseTablePrimaryKey.getDatabaseId(), houseTablePrimaryKey.getTableId()); + } +} diff --git a/tables-test-fixtures/src/main/java/com/linkedin/openhouse/tablestest/SpringH2TestApplication.java b/tables-test-fixtures/src/main/java/com/linkedin/openhouse/tablestest/SpringH2TestApplication.java index 394d8f7e..d38acd0e 100644 --- a/tables-test-fixtures/src/main/java/com/linkedin/openhouse/tablestest/SpringH2TestApplication.java +++ b/tables-test-fixtures/src/main/java/com/linkedin/openhouse/tablestest/SpringH2TestApplication.java @@ -1,14 +1,8 @@ package com.linkedin.openhouse.tablestest; -import com.linkedin.openhouse.cluster.storage.filesystem.FsStorageProvider; -import java.io.IOException; -import java.io.UncheckedIOException; import java.util.function.Consumer; import java.util.function.Supplier; import org.apache.hadoop.fs.Path; -import org.apache.hadoop.fs.permission.FsAction; -import org.apache.hadoop.fs.permission.FsPermission; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.SpringApplication; import org.springframework.boot.actuate.autoconfigure.security.servlet.ManagementWebSecurityAutoConfiguration; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; @@ -24,12 +18,14 @@ basePackages = { "com.linkedin.openhouse.tables.api", "com.linkedin.openhouse.tables.audit", + "com.linkedin.openhouse.tables.toggle", "com.linkedin.openhouse.tables.authorization", "com.linkedin.openhouse.tables.dto.mapper", "com.linkedin.openhouse.tables.utils", "com.linkedin.openhouse.tables.controller", "com.linkedin.openhouse.tables.services", "com.linkedin.openhouse.tables.config", + "com.linkedin.openhouse.tables.toggle.repository", "com.linkedin.openhouse.internal.catalog", "com.linkedin.openhouse.cluster.configs", "com.linkedin.openhouse.cluster.storage", @@ -45,7 +41,6 @@ @EnableAutoConfiguration( exclude = {SecurityAutoConfiguration.class, ManagementWebSecurityAutoConfiguration.class}) public class SpringH2TestApplication { - @Autowired protected FsStorageProvider fsStorageProvider; public static void main(String[] args) { SpringApplication.run(SpringH2TestApplication.class, args); @@ -60,14 +55,7 @@ public static void main(String[] args) { @Primary Consumer> provideTestFileSecurer() { return pathSupplier -> { - try { - fsStorageProvider - .storageClient() - .setPermission( - pathSupplier.get(), new FsPermission(FsAction.ALL, FsAction.NONE, FsAction.NONE)); - } catch (IOException ioe) { - throw new UncheckedIOException(ioe); - } + // This is a no-op Consumer. It does nothing with the supplied Path. }; } }