Skip to content

Support persisting TableMetadata in the metastore #433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 97 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
97 commits
Select commit Hold shift + click to select a range
6c90a3d
wip
eric-maynard Nov 6, 2024
bda008e
first real commit
eric-maynard Nov 7, 2024
c38a01d
cleaning up
eric-maynard Nov 7, 2024
3a69953
tests fixed
eric-maynard Nov 7, 2024
ca856ed
begin refactoring
eric-maynard Nov 7, 2024
779d912
wire up to loadTable
eric-maynard Nov 7, 2024
b4c5335
autolint
eric-maynard Nov 7, 2024
6e23be4
logic to purge old records
eric-maynard Nov 7, 2024
5805679
autolint
eric-maynard Nov 7, 2024
f32a82c
stable, disabled
eric-maynard Nov 7, 2024
249ecac
passing when enabled
eric-maynard Nov 7, 2024
0db85eb
stable
eric-maynard Nov 7, 2024
6f0d9df
autolint
eric-maynard Nov 7, 2024
c0ea191
resolve conflicts; pull main
eric-maynard Nov 7, 2024
f250916
autolint
eric-maynard Nov 7, 2024
dd87d42
oops
eric-maynard Nov 7, 2024
c46eb03
add try/catch
eric-maynard Nov 7, 2024
1e1b953
autolint
eric-maynard Nov 7, 2024
a164807
added drop-on-drop
eric-maynard Nov 8, 2024
6ed3f97
autolint
eric-maynard Nov 8, 2024
35b852f
refactor to remove new entity
eric-maynard Nov 12, 2024
0b3df37
autolint
eric-maynard Nov 12, 2024
5f7c74f
fixes
eric-maynard Nov 12, 2024
d99a796
autolint
eric-maynard Nov 12, 2024
f1023ec
stable
eric-maynard Nov 12, 2024
045d93d
autolint
eric-maynard Nov 12, 2024
20edca5
polish
eric-maynard Nov 12, 2024
f189e98
one fix
eric-maynard Nov 12, 2024
5e90ba2
autolint
eric-maynard Nov 12, 2024
fbdce6a
?
eric-maynard Nov 12, 2024
f1461e5
more widespread use of loadTableMetadata
eric-maynard Nov 14, 2024
c5081d5
autolint
eric-maynard Nov 14, 2024
452c5a1
persist on write
eric-maynard Nov 14, 2024
b85ad92
autolint
eric-maynard Nov 14, 2024
fde3910
debugging
eric-maynard Nov 14, 2024
34b9b0f
fix failing test, add TODO
eric-maynard Nov 14, 2024
6bc07c8
debugging
eric-maynard Nov 14, 2024
a9dcaa1
fix current metadata check
eric-maynard Nov 14, 2024
9b66016
autolint
eric-maynard Nov 14, 2024
52b82f7
fix
eric-maynard Nov 14, 2024
ee3f401
one change per review
eric-maynard Nov 15, 2024
69c2ca5
fixes per review
eric-maynard Nov 17, 2024
7b3e68d
autolint
eric-maynard Nov 17, 2024
0b5291e
add a comment per review; make check more clear
eric-maynard Nov 19, 2024
5488d76
autolint
eric-maynard Nov 19, 2024
64e8d2b
fix method name
eric-maynard Nov 19, 2024
5eca203
autolint
eric-maynard Nov 19, 2024
7f89a24
remove check
eric-maynard Nov 21, 2024
9829486
autolint
eric-maynard Nov 21, 2024
73f5dd2
defer serialization
eric-maynard Nov 25, 2024
59361ed
bounded serde
eric-maynard Nov 25, 2024
c67492f
autolint
eric-maynard Nov 25, 2024
85ec25b
no more exceptions
eric-maynard Nov 25, 2024
424b7d9
comment
eric-maynard Nov 25, 2024
94b5864
Merge branch 'main' of github.com:apache/polaris into metadata-cache
eric-maynard Nov 25, 2024
a5fe3c1
fix
eric-maynard Nov 25, 2024
0b152f7
autolint
eric-maynard Nov 25, 2024
3584842
stable
eric-maynard Nov 25, 2024
8d704eb
fix
eric-maynard Nov 25, 2024
5c007bc
fixing conflicts
eric-maynard Apr 28, 2025
37f4d7a
start fixing conflicts
eric-maynard Apr 29, 2025
3cd4b6a
autolint
eric-maynard Apr 29, 2025
5d7d169
stable again
eric-maynard Apr 29, 2025
085f962
Merge branch 'main' of github.com:apache/polaris into metadata-cache
eric-maynard Apr 29, 2025
f6afa2e
missing callsite
eric-maynard Apr 30, 2025
8739bfe
autolint
eric-maynard Apr 30, 2025
25a1c14
Merge branch 'main' of github.com:apache/polaris into metadata-cache
eric-maynard Apr 30, 2025
4f19dac
fixes
eric-maynard Apr 30, 2025
55dd50d
autolint
eric-maynard Apr 30, 2025
7858aa6
experiment with direct json streaming
eric-maynard Apr 30, 2025
d907378
test fix
eric-maynard Apr 30, 2025
e434e21
wip
eric-maynard Apr 30, 2025
422ef42
autolint
eric-maynard Apr 30, 2025
a829b07
fixes
eric-maynard Apr 30, 2025
a4e1781
autolint
eric-maynard Apr 30, 2025
efddd27
snapshot filtering
eric-maynard Apr 30, 2025
d78fdaf
autolint
eric-maynard Apr 30, 2025
efb3be2
fix
eric-maynard May 1, 2025
b98e845
stable
eric-maynard May 1, 2025
fea135d
improve null check
eric-maynard May 14, 2025
99e2a37
add a test
eric-maynard May 14, 2025
2908207
autolint
eric-maynard May 14, 2025
cbf1b86
changes per review
eric-maynard May 16, 2025
d8ba272
autolint
eric-maynard May 16, 2025
3644626
resolve conflicts
eric-maynard May 16, 2025
1d5c349
Merge branch 'bugfix-safe-property' of github.com-oss:eric-maynard/po…
eric-maynard May 16, 2025
8e98c17
re-stabilize
eric-maynard May 16, 2025
a7d5ebd
constnats
eric-maynard May 16, 2025
93da278
Merge branch 'main' of github.com:apache/polaris into metadata-cache
eric-maynard May 16, 2025
1ee7260
polish
eric-maynard May 16, 2025
35fb809
another change
eric-maynard May 16, 2025
6d8d703
autolint
eric-maynard May 16, 2025
eb22901
fixes
eric-maynard May 16, 2025
0f4125d
comments
eric-maynard May 22, 2025
ca85168
another comment
eric-maynard May 22, 2025
fb17414
pull main
eric-maynard May 22, 2025
17b950f
autolint
eric-maynard May 22, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.apache.polaris.core.config;

import java.util.Optional;
import java.util.function.Function;

/**
* Internal configuration flags for non-feature behavior changes in Polaris. These flags control
Expand All @@ -37,8 +38,9 @@ protected BehaviorChangeConfiguration(
String description,
T defaultValue,
Optional<String> catalogConfig,
Optional<String> catalogConfigUnsafe) {
super(key, description, defaultValue, catalogConfig, catalogConfigUnsafe);
Optional<String> catalogConfigUnsafe,
Optional<Function<T, Boolean>> validation) {
super(key, description, defaultValue, catalogConfig, catalogConfigUnsafe, validation);
}

public static final BehaviorChangeConfiguration<Boolean> VALIDATE_VIEW_LOCATION_OVERLAP =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import java.util.List;
import java.util.Optional;
import java.util.function.Function;
import org.apache.polaris.core.admin.model.StorageConfigInfo;
import org.apache.polaris.core.connection.ConnectionType;
import org.apache.polaris.core.context.CallContext;
Expand All @@ -38,8 +39,14 @@ protected FeatureConfiguration(
String description,
T defaultValue,
Optional<String> catalogConfig,
Optional<String> catalogConfigUnsafe) {
super(key, description, defaultValue, catalogConfig, catalogConfigUnsafe);
Optional<String> catalogConfigUnsafe,
Optional<Function<T, Boolean>> validation) {
super(key, description, defaultValue, catalogConfig, catalogConfigUnsafe, validation);
}

public static final class Constants {
public static final int METADATA_CACHE_MAX_BYTES_NO_CACHING = 0;
public static final int METADATA_CACHE_MAX_BYTES_INFINITE_CACHING = -1;
}

/**
Expand Down Expand Up @@ -287,4 +294,25 @@ public static void enforceFeatureEnabledOrThrow(
+ "This should only be set to 'true' for tests!")
.defaultValue(false)
.buildFeatureConfiguration();

public static final PolarisConfiguration<Integer> METADATA_CACHE_MAX_BYTES =
PolarisConfiguration.<Integer>builder()
.key("METADATA_CACHE_MAX_BYTES")
.catalogConfig("polaris.config.metadata-cache-max-bytes")
.description(
"If nonzero, the approximate max size a table's metadata can be in order to be cached in the persistence"
+ " layer. If zero, no metadata will be cached or served from the cache. If -1, all metadata"
+ " will be cached.")
.defaultValue(Constants.METADATA_CACHE_MAX_BYTES_NO_CACHING)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocker: It's reasonable to turn it off by default. Do we have a recommended size so that users/admins could use it without additional evaluation work?

.validation(value -> value >= -1)
.buildFeatureConfiguration();

public static final PolarisConfiguration<Boolean> ALWAYS_FILTER_SNAPSHOTS =
PolarisConfiguration.<Boolean>builder()
.key("ALWAYS_FILTER_SNAPSHOTS")
.description(
"If set, Polaris will always attempt to filter snapshots from a LoadTableResponse even when "
+ "doing so requires additional serialization of the TableMetadata")
.defaultValue(true)
.buildFeatureConfiguration();
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public abstract class PolarisConfiguration<T> {
private final Optional<String> catalogConfigImpl;
private final Optional<String> catalogConfigUnsafeImpl;
private final Class<T> typ;
private final Optional<Function<T, Boolean>> validation;

/** catalog configs are expected to start with this prefix */
private static final String SAFE_CATALOG_CONFIG_PREFIX = "polaris.config.";
Expand Down Expand Up @@ -90,13 +91,18 @@ protected PolarisConfiguration(
String description,
T defaultValue,
Optional<String> catalogConfig,
Optional<String> catalogConfigUnsafe) {
Optional<String> catalogConfigUnsafe,
Optional<Function<T, Boolean>> validation) {
this.key = key;
this.description = description;
this.defaultValue = defaultValue;
this.catalogConfigImpl = catalogConfig;
this.catalogConfigUnsafeImpl = catalogConfigUnsafe;
this.typ = (Class<T>) defaultValue.getClass();
this.validation = validation;

// Force validation:
apply(defaultValue);
}

public boolean hasCatalogConfig() {
Expand All @@ -110,6 +116,22 @@ public String catalogConfig() {
"Attempted to read a catalog config key from a configuration that doesn't have one."));
}

T apply(Object value) {
T result = this.typ.cast(value);
validate(result);
return result;
}

private void validate(T value) {
this.validation.ifPresent(
v -> {
if (!v.apply(value)) {
throw new IllegalArgumentException(
String.format("Configuration %s has invalid value %s", key, defaultValue));
}
});
}

public boolean hasCatalogConfigUnsafe() {
return catalogConfigUnsafeImpl.isPresent();
}
Expand All @@ -121,16 +143,13 @@ public String catalogConfigUnsafe() {
"Attempted to read an unsafe catalog config key from a configuration that doesn't have one."));
}

T cast(Object value) {
return this.typ.cast(value);
}

public static class Builder<T> {
private String key;
private String description;
private T defaultValue;
private Optional<String> catalogConfig = Optional.empty();
private Optional<String> catalogConfigUnsafe = Optional.empty();
private Optional<Function<T, Boolean>> validation = Optional.empty();

public Builder<T> key(String key) {
this.key = key;
Expand Down Expand Up @@ -162,6 +181,11 @@ public Builder<T> catalogConfig(String catalogConfig) {
return this;
}

public Builder<T> validation(Function<T, Boolean> validation) {
this.validation = Optional.of(validation);
return this;
}

/**
* Used to support backwards compatability before there were reserved properties. Usage of this
* method should be removed over time.
Expand All @@ -177,7 +201,7 @@ public Builder<T> catalogConfigUnsafe(String catalogConfig) {
return this;
}

private void validateOrThrow() {
private void validateFieldsOrThrow() {
if (key == null || description == null || defaultValue == null) {
throw new IllegalArgumentException("key, description, and defaultValue are required");
}
Expand All @@ -187,23 +211,23 @@ private void validateOrThrow() {
}

public FeatureConfiguration<T> buildFeatureConfiguration() {
validateOrThrow();
validateFieldsOrThrow();
FeatureConfiguration<T> config =
new FeatureConfiguration<>(
key, description, defaultValue, catalogConfig, catalogConfigUnsafe);
key, description, defaultValue, catalogConfig, catalogConfigUnsafe, validation);
PolarisConfiguration.registerConfiguration(config);
return config;
}

public BehaviorChangeConfiguration<T> buildBehaviorChangeConfiguration() {
validateOrThrow();
validateFieldsOrThrow();
if (catalogConfig.isPresent() || catalogConfigUnsafe.isPresent()) {
throw new IllegalArgumentException(
"catalog configs are not valid for behavior change configs");
}
BehaviorChangeConfiguration<T> config =
new BehaviorChangeConfiguration<>(
key, description, defaultValue, catalogConfig, catalogConfigUnsafe);
key, description, defaultValue, catalogConfig, catalogConfigUnsafe, validation);
PolarisConfiguration.registerConfiguration(config);
return config;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,17 @@ public interface PolarisConfigurationStore {
}

if (config.defaultValue instanceof Boolean) {
return config.cast(Boolean.valueOf(String.valueOf(value)));
return config.apply(Boolean.valueOf(String.valueOf(value)));
} else if (config.defaultValue instanceof Integer) {
return config.cast(Integer.valueOf(String.valueOf(value)));
return config.apply(Integer.valueOf(String.valueOf(value)));
} else if (config.defaultValue instanceof Long) {
return config.cast(Long.valueOf(String.valueOf(value)));
return config.apply(Long.valueOf(String.valueOf(value)));
} else if (config.defaultValue instanceof Double) {
return config.cast(Double.valueOf(String.valueOf(value)));
return config.apply(Double.valueOf(String.valueOf(value)));
} else if (config.defaultValue instanceof List<?>) {
return config.cast(new ArrayList<>((List<?>) value));
return config.apply(new ArrayList<>((List<?>) value));
} else {
return config.cast(value);
return config.apply(value);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@ public class IcebergTableLikeEntity extends TableLikeEntity {
// of the internalProperties JSON file.
public static final String METADATA_LOCATION_KEY = "metadata-location";

// For applicable types, this key on the "internalProperties" map will return the content of the
// metadata.json file located at `METADATA_CACHE_LOCATION_KEY`
public static final String METADATA_CACHE_CONTENT_KEY = "metadata-cache-content";

// For applicable types, this key on the "internalProperties" map will return the location of the
// `metadata.json` that is cached in `METADATA_CACHE_CONTENT_KEY`. This will often match the
// current metadata location in `METADATA_LOCATION_KEY`; if it does not the cache is invalid
public static final String METADATA_CACHE_LOCATION_KEY = "metadata-cache-location";

public static final String USER_SPECIFIED_WRITE_DATA_LOCATION_KEY = "write.data.path";
public static final String USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY = "write.metadata.path";

Expand All @@ -60,6 +69,16 @@ public String getMetadataLocation() {
return getInternalPropertiesAsMap().get(METADATA_LOCATION_KEY);
}

@JsonIgnore
public String getMetadataCacheContent() {
return getInternalPropertiesAsMap().get(METADATA_CACHE_CONTENT_KEY);
}

@JsonIgnore
public String getMetadataCacheLocation() {
return getInternalPropertiesAsMap().get(METADATA_CACHE_LOCATION_KEY);
}

@JsonIgnore
public Optional<Long> getLastAdmittedNotificationTimestamp() {
return Optional.ofNullable(
Expand Down Expand Up @@ -114,6 +133,12 @@ public Builder setMetadataLocation(String location) {
return this;
}

public Builder setMetadataContent(String location, String content) {
internalProperties.put(METADATA_CACHE_LOCATION_KEY, location);
internalProperties.put(METADATA_CACHE_CONTENT_KEY, content);
return this;
}

public Builder setLastNotificationTimestamp(long timestamp) {
internalProperties.put(LAST_ADMITTED_NOTIFICATION_TIMESTAMP_KEY, String.valueOf(timestamp));
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public class EntityWeigher implements Weigher<Long, ResolvedPolarisEntity> {
private static final int APPROXIMATE_ENTITY_OVERHEAD = 1000;

/* Represents the amount of bytes that a character is expected to take up */
private static final int APPROXIMATE_BYTES_PER_CHAR = 3;
public static final int APPROXIMATE_BYTES_PER_CHAR = 3;

/** Singleton instance */
private static final EntityWeigher instance = new EntityWeigher();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public InMemoryEntityCacheTest() {
callCtx = new PolarisCallContext(metaStore, diagServices);
metaStoreManager = new TransactionalMetaStoreManagerImpl();

// bootstrap the mata store with our test schema
// bootstrap the metastore with our test schema
tm = new PolarisTestMetaStoreManager(metaStoreManager, callCtx);
tm.testCreateTestCatalog();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public void testConfigsCanBeCastedFromString() {
}

@Test
public void testInvalidCastThrowsException() {
public void testInvalidApplyThrowsException() {
// Bool not included because Boolean.valueOf turns non-boolean strings to false
List<PolarisConfiguration<?>> configs =
List.of(buildConfig("int2", 12), buildConfig("long2", 34L), buildConfig("double2", 5.6D));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.exceptions.ForbiddenException;
import org.apache.iceberg.io.FileIO;
import org.apache.iceberg.rest.RESTResponse;
import org.apache.iceberg.rest.requests.CommitTransactionRequest;
import org.apache.iceberg.rest.requests.CreateNamespaceRequest;
import org.apache.iceberg.rest.requests.CreateTableRequest;
Expand All @@ -48,6 +49,7 @@
import org.apache.iceberg.rest.requests.RenameTableRequest;
import org.apache.iceberg.rest.requests.UpdateNamespacePropertiesRequest;
import org.apache.iceberg.rest.requests.UpdateTableRequest;
import org.apache.iceberg.rest.responses.LoadTableResponse;
import org.apache.iceberg.view.ImmutableSQLViewRepresentation;
import org.apache.iceberg.view.ImmutableViewVersion;
import org.apache.polaris.core.admin.model.CreateCatalogRequest;
Expand Down Expand Up @@ -757,6 +759,11 @@ public void testCreateTableStagedWithWriteDelegationInsufficientPermissions() {
});
}

private static String getMetadataLocation(RESTResponse resp) {
final String metadataLocation = ((LoadTableResponse) resp).metadataLocation();
return metadataLocation;
}

@Test
public void testRegisterTableAllSufficientPrivileges() {
Assertions.assertThat(
Expand All @@ -770,7 +777,7 @@ public void testRegisterTableAllSufficientPrivileges() {

// To get a handy metadata file we can use one from another table.
// to avoid overlapping directories, drop the original table and recreate it via registerTable
final String metadataLocation = newWrapper().loadTable(TABLE_NS1_1, "all").metadataLocation();
final String metadataLocation = getMetadataLocation(newWrapper().loadTable(TABLE_NS1_1, "all"));
newWrapper(Set.of(PRINCIPAL_ROLE2)).dropTableWithoutPurge(TABLE_NS1_1);

final RegisterTableRequest registerRequest =
Expand Down Expand Up @@ -808,7 +815,7 @@ public void testRegisterTableInsufficientPermissions() {
.isTrue();

// To get a handy metadata file we can use one from another table.
final String metadataLocation = newWrapper().loadTable(TABLE_NS1_1, "all").metadataLocation();
final String metadataLocation = getMetadataLocation(newWrapper().loadTable(TABLE_NS1_1, "all"));

final RegisterTableRequest registerRequest =
new RegisterTableRequest() {
Expand Down
Loading
Loading