Skip to content

Commit

Permalink
Add flag to only fetch metadata location for iceberg tables (#508)
Browse files Browse the repository at this point in the history
  • Loading branch information
raveeram authored May 3, 2022
1 parent d0cd30b commit 186aaa1
Show file tree
Hide file tree
Showing 12 changed files with 140 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,33 @@ default TableDto getTable(
* @param includeInfoDetails true if the more info details to be included
* @return table
*/
default TableDto getTable(
String catalogName,
String databaseName,
String tableName,
Boolean includeInfo,
Boolean includeDefinitionMetadata,
Boolean includeDataMetadata,
Boolean includeInfoDetails
) {
return getTable(catalogName, databaseName, tableName, includeInfo,
includeDefinitionMetadata, includeDataMetadata, includeInfoDetails, false);
}

/**
* Get the table.
*
* @param catalogName catalog name
* @param databaseName database name
* @param tableName table name.
* @param includeInfo true if the details need to be included
* @param includeDefinitionMetadata true if the definition metadata to be included
* @param includeDataMetadata true if the data metadata to be included
* @param includeInfoDetails true if the more info details to be included
* @param includeMetadataLocationOnly true if only metadata location needs to be included.
* All other flags are ignored if this is set to true.
* @return table
*/
@GET
@Path("catalog/{catalog-name}/database/{database-name}/table/{table-name}")
@Consumes(MediaType.APPLICATION_JSON)
Expand All @@ -303,7 +330,10 @@ TableDto getTable(
Boolean includeDataMetadata,
@DefaultValue("false")
@QueryParam("includeInfoDetails")
Boolean includeInfoDetails
Boolean includeInfoDetails,
@DefaultValue("false")
@QueryParam("includeMetadataLocationOnly")
Boolean includeMetadataLocationOnly
);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,42 @@ default TableDto getTable(
* @param includeInfoDetails true if the more info details to be included
* @return table
*/
TableDto getTable(
default TableDto getTable(
final String catalogName,
final String databaseName,
final String tableName,
final boolean includeInfo,
final boolean includeDefinitionMetadata,
final boolean includeDataMetadata,
final boolean includeInfoDetails
) {
return getTable(catalogName, databaseName, tableName, includeInfo, includeDefinitionMetadata,
includeDataMetadata, includeInfoDetails, false);
}

/**
* Get the table.
*
* @param catalogName catalog name
* @param databaseName database name
* @param tableName table name.
* @param includeInfo true if the details need to be included
* @param includeDefinitionMetadata true if the definition metadata to be included
* @param includeDataMetadata true if the data metadata to be included
* @param includeInfoDetails true if the more info details to be included
* @param includeMetadataLocationOnly true if only metadata location needs to be included.
* All other flags are ignored
* @return table
*/
TableDto getTable(
final String catalogName,
final String databaseName,
final String tableName,
final boolean includeInfo,
final boolean includeDefinitionMetadata,
final boolean includeDataMetadata,
final boolean includeInfoDetails,
final boolean includeMetadataLocationOnly
);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public class ConnectorRequestContext {
private long timestamp;
private String userName;
private boolean includeMetadata;
private boolean includeMetadataLocationOnly;
//TODO: Move this to a response object.
private boolean ignoreErrorsAfterUpdate;
}
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,7 @@ public interface Config {

/**
* Whether the table alias is enabled.
*
* @return True if it is.
*/
boolean isTableAliasEnabled();
Expand Down Expand Up @@ -556,5 +557,12 @@ public interface Config {
* @return True if it should be blocked.
*/
boolean disablePartitionDefinitionMetadata();

/**
* Whether the request flag to only fetch the iceberg metadata location should be respected.
*
* @return True if it should be.
*/
boolean shouldFetchOnlyMetadataLocationEnabled();
}

Original file line number Diff line number Diff line change
Expand Up @@ -640,4 +640,9 @@ public boolean isIcebergPreviousMetadataLocationCheckEnabled() {
public boolean disablePartitionDefinitionMetadata() {
return this.metacatProperties.getDefinition().getMetadata().isDisablePartitionDefinitionMetadata();
}

@Override
public boolean shouldFetchOnlyMetadataLocationEnabled() {
return this.metacatProperties.getHive().getIceberg().isShouldFetchOnlyMetadataLocationEnabled();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ public static class Iceberg {
/*iceberg://<db-name.table-name>/<partition>/snapshot_time=<dateCreated> */
private String partitionUriScheme = "iceberg";
private boolean isIcebergPreviousMetadataLocationCheckEnabled = true;
private boolean isShouldFetchOnlyMetadataLocationEnabled = true;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,11 @@ public TableInfo get(final ConnectorRequestContext requestContext, final Qualifi
if (!connectorContext.getConfig().isIcebergEnabled() || !HiveTableUtil.isIcebergTable(info)) {
return info;
}
// Return the iceberg table with just the metadata location included.
if (connectorContext.getConfig().shouldFetchOnlyMetadataLocationEnabled()
&& requestContext.isIncludeMetadataLocationOnly()) {
return info;
}
final String tableLoc = HiveTableUtil.getIcebergTableMetadataLocation(info);
final TableInfo result = hiveConnectorFastTableServiceProxy.getIcebergTable(name, tableLoc, info,
requestContext.isIncludeMetadata(), connectorContext.getConfig().isIcebergCacheEnabled());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.netflix.metacat.connector.polaris;

import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.netflix.metacat.common.QualifiedName;
import com.netflix.metacat.common.dto.Pageable;
import com.netflix.metacat.common.dto.Sort;
Expand Down Expand Up @@ -31,6 +32,7 @@
import org.springframework.dao.DataIntegrityViolationException;

import javax.annotation.Nullable;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -146,6 +148,14 @@ public TableInfo get(final ConnectorRequestContext requestContext, final Qualifi
.orElseThrow(() -> new TableNotFoundException(name));
final TableInfo info = polarisTableMapper.toInfo(polarisTableEntity);
final String tableLoc = HiveTableUtil.getIcebergTableMetadataLocation(info);
// Return the iceberg table with just the metadata location included if requested.
if (connectorContext.getConfig().shouldFetchOnlyMetadataLocationEnabled()
&& requestContext.isIncludeMetadataLocationOnly()) {
return TableInfo.builder()
.metadata(Maps.newHashMap(info.getMetadata()))
.fields(Collections.emptyList())
.build();
}
return getIcebergTable(name, tableLoc, info,
requestContext.isIncludeMetadata(), connectorContext.getConfig().isIcebergCacheEnabled());
} catch (TableNotFoundException | IllegalArgumentException exception) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,11 @@ class MetacatSmokeSpec extends Specification {
then:
noExceptionThrown()
when:
def tableMetadataOnly = api.getTable(catalogName, databaseName, tableName, true, false, false, false, true)
then:
tableMetadataOnly.getFields().size() == 0
tableMetadataOnly.getMetadata().get('metadata_location') != null
when:
FileUtils.moveFile(metadataFile, new File(metadataFile.getAbsolutePath() + '1'))
api.getTable(catalogName, databaseName, tableName, true, false, false)
then:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -669,33 +669,38 @@ public TableDto getTable(
@RequestParam(name = "includeDataMetadata", defaultValue = "true") final boolean includeDataMetadata,
@ApiParam(value = "Whether to include more info details to the response. This value is considered only if "
+ "includeInfo is true.")
@RequestParam(name = "includeInfoDetails", defaultValue = "false") final boolean includeInfoDetails
@RequestParam(name = "includeInfoDetails", defaultValue = "false") final boolean includeInfoDetails,
@ApiParam(value = "Whether to include only the metadata location in the response")
@RequestParam(
name = "includeMetadataLocationOnly",
defaultValue = "false") final boolean includeMetadataLocationOnly
) {
final Supplier<QualifiedName> qualifiedNameSupplier =
() -> QualifiedName.ofTable(catalogName, databaseName, tableName);
() -> QualifiedName.ofTable(catalogName, databaseName, tableName);
final QualifiedName name = this.requestWrapper.qualifyName(qualifiedNameSupplier);
return this.requestWrapper.processRequest(
name,
"getTable",
() -> {
final Optional<TableDto> table = this.tableService.get(
name,
GetTableServiceParameters.builder()
.includeInfo(includeInfo)
.includeDefinitionMetadata(includeDefinitionMetadata)
.includeDataMetadata(includeDataMetadata)
.disableOnReadMetadataIntercetor(false)
.includeMetadataFromConnector(includeInfoDetails)
.useCache(true)
.build()
);
name,
"getTable",
() -> {
final Optional<TableDto> table = this.tableService.get(
name,
GetTableServiceParameters.builder()
.includeInfo(includeInfo)
.includeDefinitionMetadata(includeDefinitionMetadata)
.includeDataMetadata(includeDataMetadata)
.disableOnReadMetadataIntercetor(false)
.includeMetadataFromConnector(includeInfoDetails)
.includeMetadataLocationOnly(includeMetadataLocationOnly)
.useCache(true)
.build()
);

final TableDto tableDto = table.orElseThrow(() -> new TableNotFoundException(name));
// Set the name to whatever the request was for because
// for aliases, this could've been set to the original name
tableDto.setName(qualifiedNameSupplier.get());
return tableDto;
}
final TableDto tableDto = table.orElseThrow(() -> new TableNotFoundException(name));
// Set the name to whatever the request was for because
// for aliases, this could've been set to the original name
tableDto.setName(qualifiedNameSupplier.get());
return tableDto;
}
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,5 @@ public class GetTableServiceParameters {
private final boolean disableOnReadMetadataIntercetor;
private final boolean useCache;
private final boolean includeMetadataFromConnector;
private final boolean includeMetadataLocationOnly;
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.springframework.cache.annotation.CacheConfig;
import org.springframework.cache.annotation.CacheEvict;
import org.springframework.cache.annotation.Cacheable;
import org.springframework.cache.annotation.Caching;

import java.util.Collection;
import java.util.List;
Expand Down Expand Up @@ -77,7 +78,10 @@ public void create(final QualifiedName name, final TableInfo tableInfo) {
* Calls the connector table service delete method.
* @param name table name
*/
@CacheEvict(key = "'table.' + #name")
@Caching(evict = {
@CacheEvict(key = "'table.' + #name"),
@CacheEvict(key = "'table.metadataLocationOnly.' + #name")
})
public void delete(final QualifiedName name) {
final MetacatRequestContext metacatRequestContext = MetacatContextManager.getContext();
final ConnectorTableService service = connectorManager.getTableService(name);
Expand All @@ -97,13 +101,16 @@ public void delete(final QualifiedName name) {
* @param useCache true, if table can be retrieved from cache
* @return table dto
*/
@Cacheable(key = "'table.' + #name", condition = "#useCache")
@Cacheable(key = "{#getTableServiceParameters.isIncludeMetadataLocationOnly() ? "
+ "'table.metadataLocationOnly.' + #name : 'table.' + #name}", condition = "#useCache")
public TableInfo get(final QualifiedName name,
final GetTableServiceParameters getTableServiceParameters,
final boolean useCache) {
final MetacatRequestContext metacatRequestContext = MetacatContextManager.getContext();
final ConnectorRequestContext connectorRequestContext = converterUtil.toConnectorContext(metacatRequestContext);
connectorRequestContext.setIncludeMetadata(getTableServiceParameters.isIncludeMetadataFromConnector());
connectorRequestContext.setIncludeMetadataLocationOnly(
getTableServiceParameters.isIncludeMetadataLocationOnly());
final ConnectorTableService service = connectorManager.getTableService(name);
return service.get(connectorRequestContext, name);
}
Expand All @@ -114,7 +121,10 @@ public TableInfo get(final QualifiedName name,
* @param newName new table name
* @param isMView true, if the object is a view
*/
@CacheEvict(key = "'table.' + #oldName")
@Caching(evict = {
@CacheEvict(key = "'table.' + #oldName"),
@CacheEvict(key = "'table.metadataLocationOnly.' + #oldName")
})
public void rename(
final QualifiedName oldName,
final QualifiedName newName,
Expand All @@ -138,7 +148,10 @@ public void rename(
* @param tableInfo table object
* @return true if errors after this should be ignored.
*/
@CacheEvict(key = "'table.' + #name")
@Caching(evict = {
@CacheEvict(key = "'table.' + #name"),
@CacheEvict(key = "'table.metadataLocationOnly.' + #name")
})
public boolean update(final QualifiedName name, final TableInfo tableInfo) {
final MetacatRequestContext metacatRequestContext = MetacatContextManager.getContext();
final ConnectorTableService service = connectorManager.getTableService(name);
Expand Down

0 comments on commit 186aaa1

Please sign in to comment.