You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "snazy (via GitHub)" <gi...@apache.org> on 2025/12/14 10:54:59 UTC
[PR] NoSQL: Metastore maintenance [polaris]
snazy opened a new pull request, #3268:
URL: https://github.com/apache/polaris/pull/3268
Implementation of the NoSQL meta-store maintenance implementation. It adds the meta-store specific handling to the existing NoSQL maintenance service to purge unreferenced and unneeded data from the database.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "dennishuo (via GitHub)" <gi...@apache.org>.
dennishuo commented on code in PR #3268:
URL: https://github.com/apache/polaris/pull/3268#discussion_r2621878125
##########
persistence/nosql/persistence/metastore-maintenance/src/main/java/org/apache/polaris/persistence/nosql/metastore/maintenance/CatalogsMaintenanceConfig.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.metastore.maintenance;
+
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import io.smallrye.config.ConfigMapping;
+import io.smallrye.config.WithDefault;
+import java.util.Optional;
+import org.apache.polaris.immutables.PolarisImmutable;
+
+/**
+ * Polaris stores a history of changes per kind of object (principals, principal roles, grants,
+ * immediate tasks, catalog roles and catalog state).
+ *
+ * <p>The rules are defined using a <a href="https://github.com/projectnessie/cel-java/">CEL
+ * script</a>. The default rules for all kinds of objects are to retain the history for 3 days, for
+ * the catalog state for 30 days.
+ *
+ * <p>The scripts have access to the following declared values:
+ *
+ * <ul>
+ * <li>{@code ref} (string) name of the reference
+ * <li>{@code commits} (64-bit int) number of the currently processed commit, starting at {@code
+ * 1}
+ * <li>{@code ageDays} (64-bit int) age of currently processed commit in days
+ * <li>{@code ageHours} (64-bit int) age of currently processed commit in hours
+ * <li>{@code ageMinutes} (64-bit int) age of currently processed commit in minutes
+ * </ul>
+ *
+ * <p>Scripts <em>must</em> return a {@code boolean} yielding whether the commit shall be retained.
+ * Note that maintenance-service implementations can keep the first not-to-be-retained commit.
+ *
+ * <p>Example scripts
+ *
+ * <ul>
+ * <li>{@code ageDays < 30 || commits <= 10} retains the reference history with at least 10
+ * commits and commits that are younger than 30 days
+ * <li>{@code true} retains the whole reference history
+ * <li>{@code false} retains the most recent commit
+ * </ul>
+ */
+@ConfigMapping(prefix = "polaris.persistence.maintenance.catalog")
Review Comment:
+1, I know there was some ongoing debate about config keys, but IIRC we did move ahead with the `polaris.persistence.nosql` key prefix in https://github.com/apache/polaris/pull/3135 so I'm assuming additional config keys will follow the same pattern.
##########
persistence/nosql/persistence/metastore-maintenance/src/main/java/org/apache/polaris/persistence/nosql/metastore/maintenance/CatalogsMaintenanceConfig.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.metastore.maintenance;
+
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import io.smallrye.config.ConfigMapping;
+import io.smallrye.config.WithDefault;
+import java.util.Optional;
+import org.apache.polaris.immutables.PolarisImmutable;
+
+/**
+ * Polaris stores a history of changes per kind of object (principals, principal roles, grants,
+ * immediate tasks, catalog roles and catalog state).
+ *
+ * <p>The rules are defined using a <a href="https://github.com/projectnessie/cel-java/">CEL
+ * script</a>. The default rules for all kinds of objects are to retain the history for 3 days, for
+ * the catalog state for 30 days.
+ *
+ * <p>The scripts have access to the following declared values:
+ *
+ * <ul>
+ * <li>{@code ref} (string) name of the reference
+ * <li>{@code commits} (64-bit int) number of the currently processed commit, starting at {@code
+ * 1}
+ * <li>{@code ageDays} (64-bit int) age of currently processed commit in days
+ * <li>{@code ageHours} (64-bit int) age of currently processed commit in hours
+ * <li>{@code ageMinutes} (64-bit int) age of currently processed commit in minutes
+ * </ul>
+ *
+ * <p>Scripts <em>must</em> return a {@code boolean} yielding whether the commit shall be retained.
+ * Note that maintenance-service implementations can keep the first not-to-be-retained commit.
+ *
+ * <p>Example scripts
+ *
+ * <ul>
+ * <li>{@code ageDays < 30 || commits <= 10} retains the reference history with at least 10
+ * commits and commits that are younger than 30 days
+ * <li>{@code true} retains the whole reference history
+ * <li>{@code false} retains the most recent commit
+ * </ul>
+ */
+@ConfigMapping(prefix = "polaris.persistence.maintenance.catalog")
+@JsonSerialize(as = ImmutableBuildableCatalogsMaintenanceConfig.class)
+@JsonDeserialize(as = ImmutableBuildableCatalogsMaintenanceConfig.class)
+public interface CatalogsMaintenanceConfig {
+
+ String DEFAULT_PRINCIPALS_RETAIN = "false";
+ String DEFAULT_PRINCIPAL_ROLES_RETAIN = "false";
+ String DEFAULT_GRANTS_RETAIN = "false";
+ String DEFAULT_IMMEDIATE_TASKS_RETAIN = "false";
+ String DEFAULT_CATALOGS_HISTORY_RETAIN = "false";
+ String DEFAULT_CATALOG_ROLES_RETAIN = "false";
+ String DEFAULT_CATALOG_POLICIES_RETAIN = "ageDays < 30 || commits <= 1";
+ String DEFAULT_CATALOG_STATE_RETAIN = "ageDays < 30 || commits <= 1";
Review Comment:
Is the `ageDays` condition the only thing to distinguish between "staged but not yet committed" states that are being assembled as part of a transaction and old states that were once committed/valid but are expired now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy merged PR #3268:
URL: https://github.com/apache/polaris/pull/3268
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3268:
URL: https://github.com/apache/polaris/pull/3268#issuecomment-3751986745
Related `dev` discussion: https://lists.apache.org/thread/znw87l7r1gh3l09pc0hxhj5gyl8vp28d
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #3268:
URL: https://github.com/apache/polaris/pull/3268#issuecomment-3740672334
@dimas-b , would you mind sharing links about the discussion and conclusion of consensus of the following item?
> Using https://github.com/projectnessie/cel-java/ for rules, why such rules are required in the first place ? why not use https://github.com/google/cel-java directly which is actively maintained by google developers vs using https://github.com/projectnessie/cel-java/ which has just done dependency updates this year https://github.com/projectnessie/cel-java/pulls?page=8&q=is%3Apr+is%3Aclosed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on PR #3268:
URL: https://github.com/apache/polaris/pull/3268#issuecomment-3731162734
I appreciate the additional context and the link to the user-side tool. However, my primary concern remains the introduction of new constructs—like retention expressions—that don’t currently exist in Polaris. This feels like a significant architectural shift that warrants a broader discussion on the dev list to reach project-wide consensus.
Regarding the cel-java dependency: my concern isn't its origin, but its long-term maintenance health. Given that we have diverging views on this merge (+1 vs -1), I believe the most collaborative path forward is to revert the PR for now. This would allow us to have that broader discussion on the dev list without the pressure of the code already being in the main branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3268:
URL: https://github.com/apache/polaris/pull/3268#issuecomment-3656787162
> Unclear why maintenance module should be part of Polaris repo and not Polaris tools ?
The end-to-end maintenance workflow naturally includes some "service" code and "tool / CLI" code. This PR contains only the service portion, which has to align with the NoSQL metastore implementation, so it belongs in this repository, IMHO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #3268:
URL: https://github.com/apache/polaris/pull/3268#discussion_r2617850221
##########
persistence/nosql/persistence/metastore-maintenance/src/main/java/org/apache/polaris/persistence/nosql/metastore/maintenance/CatalogRetainedIdentifier.java:
##########
@@ -0,0 +1,334 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.metastore.maintenance;
+
+import static java.lang.String.format;
+import static org.apache.polaris.persistence.nosql.api.obj.ObjRef.OBJ_REF_SERIALIZER;
+import static org.apache.polaris.persistence.nosql.coretypes.catalog.CatalogRolesObj.CATALOG_ROLES_REF_NAME_PATTERN;
+import static org.apache.polaris.persistence.nosql.coretypes.catalog.CatalogStateObj.CATALOG_STATE_REF_NAME_PATTERN;
+import static org.apache.polaris.persistence.nosql.coretypes.catalog.CatalogsObj.CATALOGS_REF_NAME;
+import static org.apache.polaris.persistence.nosql.coretypes.principals.PrincipalRolesObj.PRINCIPAL_ROLES_REF_NAME;
+import static org.apache.polaris.persistence.nosql.coretypes.principals.PrincipalsObj.PRINCIPALS_REF_NAME;
+import static org.apache.polaris.persistence.nosql.coretypes.realm.ImmediateTasksObj.IMMEDIATE_TASKS_REF_NAME;
+import static org.apache.polaris.persistence.nosql.coretypes.realm.PolicyMapping.POLICY_MAPPING_SERIALIZER;
+import static org.apache.polaris.persistence.nosql.coretypes.realm.PolicyMappingsObj.POLICY_MAPPINGS_REF_NAME;
+import static org.apache.polaris.persistence.nosql.coretypes.realm.RealmGrantsObj.REALM_GRANTS_REF_NAME;
+import static org.apache.polaris.persistence.nosql.coretypes.realm.RootObj.ROOT_REF_NAME;
+import static org.apache.polaris.persistence.nosql.metastore.maintenance.CatalogsMaintenanceConfig.DEFAULT_CATALOGS_HISTORY_RETAIN;
+import static org.apache.polaris.persistence.nosql.metastore.maintenance.CatalogsMaintenanceConfig.DEFAULT_CATALOG_POLICIES_RETAIN;
+import static org.apache.polaris.persistence.nosql.metastore.maintenance.CatalogsMaintenanceConfig.DEFAULT_CATALOG_ROLES_RETAIN;
+import static org.apache.polaris.persistence.nosql.metastore.maintenance.CatalogsMaintenanceConfig.DEFAULT_CATALOG_STATE_RETAIN;
+import static org.apache.polaris.persistence.nosql.metastore.maintenance.CatalogsMaintenanceConfig.DEFAULT_GRANTS_RETAIN;
+import static org.apache.polaris.persistence.nosql.metastore.maintenance.CatalogsMaintenanceConfig.DEFAULT_IMMEDIATE_TASKS_RETAIN;
+import static org.apache.polaris.persistence.nosql.metastore.maintenance.CatalogsMaintenanceConfig.DEFAULT_PRINCIPALS_RETAIN;
+import static org.apache.polaris.persistence.nosql.metastore.maintenance.CatalogsMaintenanceConfig.DEFAULT_PRINCIPAL_ROLES_RETAIN;
+
+import jakarta.annotation.Nonnull;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.inject.Inject;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Consumer;
+import java.util.function.Function;
+import org.apache.polaris.ids.api.MonotonicClock;
+import org.apache.polaris.maintenance.cel.CelReferenceContinuePredicate;
+import org.apache.polaris.persistence.nosql.api.exceptions.ReferenceNotFoundException;
+import org.apache.polaris.persistence.nosql.api.index.IndexContainer;
+import org.apache.polaris.persistence.nosql.api.index.IndexKey;
+import org.apache.polaris.persistence.nosql.api.obj.BaseCommitObj;
+import org.apache.polaris.persistence.nosql.api.obj.ObjRef;
+import org.apache.polaris.persistence.nosql.coretypes.ContainerObj;
+import org.apache.polaris.persistence.nosql.coretypes.catalog.CatalogObj;
+import org.apache.polaris.persistence.nosql.coretypes.catalog.CatalogRolesObj;
+import org.apache.polaris.persistence.nosql.coretypes.catalog.CatalogStateObj;
+import org.apache.polaris.persistence.nosql.coretypes.catalog.CatalogsObj;
+import org.apache.polaris.persistence.nosql.coretypes.principals.PrincipalRolesObj;
+import org.apache.polaris.persistence.nosql.coretypes.principals.PrincipalsObj;
+import org.apache.polaris.persistence.nosql.coretypes.realm.ImmediateTasksObj;
+import org.apache.polaris.persistence.nosql.coretypes.realm.PolicyMappingsObj;
+import org.apache.polaris.persistence.nosql.coretypes.realm.RealmGrantsObj;
+import org.apache.polaris.persistence.nosql.coretypes.realm.RootObj;
+import org.apache.polaris.persistence.nosql.maintenance.spi.PerRealmRetainedIdentifier;
+import org.apache.polaris.persistence.nosql.maintenance.spi.RetainedCollector;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+class CatalogRetainedIdentifier implements PerRealmRetainedIdentifier {
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(CatalogRetainedIdentifier.class);
+
+ private final CatalogsMaintenanceConfig catalogsMaintenanceConfig;
+ private final MonotonicClock monotonicClock;
+
+ @SuppressWarnings("CdiInjectionPointsInspection")
+ @Inject
+ CatalogRetainedIdentifier(
+ CatalogsMaintenanceConfig catalogsMaintenanceConfig, MonotonicClock monotonicClock) {
+ this.catalogsMaintenanceConfig = catalogsMaintenanceConfig;
+ this.monotonicClock = monotonicClock;
+ }
+
+ @Override
+ public String name() {
+ return "Catalog data";
+ }
+
+ @Override
+ public boolean identifyRetained(@Nonnull RetainedCollector collector) {
+
+ // Note: References & objects retrieved via the `Persistence` instance returned by the
+ // `RetainedCollector` are automatically retained (no need to call collector.retain*()
+ // explicitly).
+ var persistence = collector.realmPersistence();
+
+ // per realm
+
+ // The root object is "special" (there's only one)
+ LOGGER.info("Identifying root object...");
+ ignoreReferenceNotFound(() -> persistence.fetchReferenceHead(ROOT_REF_NAME, RootObj.class));
+
+ perRealmContainer(
+ "principals",
+ PRINCIPALS_REF_NAME,
+ catalogsMaintenanceConfig.principalsRetain().orElse(DEFAULT_PRINCIPALS_RETAIN),
+ PrincipalsObj.class,
+ collector);
+
+ perRealmContainer(
+ "principal roles",
+ PRINCIPAL_ROLES_REF_NAME,
+ catalogsMaintenanceConfig.principalRolesRetain().orElse(DEFAULT_PRINCIPAL_ROLES_RETAIN),
+ PrincipalRolesObj.class,
+ collector);
+
+ perRealm(
+ "grants",
+ REALM_GRANTS_REF_NAME,
+ catalogsMaintenanceConfig.grantsRetain().orElse(DEFAULT_GRANTS_RETAIN),
+ RealmGrantsObj.class,
+ RealmGrantsObj::acls,
+ collector);
+
+ perRealmContainer(
+ "immediate tasks",
+ IMMEDIATE_TASKS_REF_NAME,
+ catalogsMaintenanceConfig.immediateTasksRetain().orElse(DEFAULT_IMMEDIATE_TASKS_RETAIN),
+ ImmediateTasksObj.class,
+ collector);
+
+ LOGGER.info("Identifying policy mappings...");
+ ignoreReferenceNotFound(
+ () -> {
+ var policyMappingsContinue =
+ new CelReferenceContinuePredicate<PolicyMappingsObj>(
+ POLICY_MAPPINGS_REF_NAME,
+ persistence,
+ catalogsMaintenanceConfig
+ .catalogPoliciesRetain()
+ .orElse(DEFAULT_CATALOG_POLICIES_RETAIN));
+ // PolicyMappings are stored _INLINE_
+ collector.refRetain(
+ POLICY_MAPPINGS_REF_NAME,
+ PolicyMappingsObj.class,
+ policyMappingsContinue,
+ policyMappingsObj ->
+ policyMappingsObj
+ .policyMappings()
+ .indexForRead(collector.realmPersistence(), POLICY_MAPPING_SERIALIZER)
+ .forEach(
+ e -> {
+ var policyMapping = e.getValue();
+ policyMapping.externalMapping().ifPresent(collector::retainObject);
+ }));
+ });
+
+ // per catalog
+
+ LOGGER.info("Identifying catalogs...");
+ ignoreReferenceNotFound(
+ () -> {
+ var catalogsHistoryContinue =
+ new CelReferenceContinuePredicate<CatalogsObj>(
+ CATALOGS_REF_NAME,
+ persistence,
+ catalogsMaintenanceConfig
+ .catalogsHistoryRetain()
+ .orElse(DEFAULT_CATALOGS_HISTORY_RETAIN));
+ var currentCatalogs = new ConcurrentHashMap<IndexKey, ObjRef>();
+ collector.refRetain(
+ CATALOGS_REF_NAME,
+ CatalogsObj.class,
+ catalogsHistoryContinue,
+ catalogs -> {
+ var allCatalogsIndex =
+ catalogs.nameToObjRef().indexForRead(persistence, OBJ_REF_SERIALIZER);
+ for (var entry : allCatalogsIndex) {
+ var catalogKey = entry.getKey();
+ var catalogObjRef = entry.getValue();
+ currentCatalogs.putIfAbsent(catalogKey, catalogObjRef);
+ }
+ collector.indexRetain(catalogs.stableIdToName());
+ });
+
+ var catalogObjs =
+ persistence.fetchMany(
+ CatalogObj.class, currentCatalogs.values().toArray(ObjRef[]::new));
+ for (var catalogObj : catalogObjs) {
+ if (catalogObj == null) {
+ // just in case...
+ continue;
+ }
+
+ perCatalog(
+ "catalog roles",
+ CATALOG_ROLES_REF_NAME_PATTERN,
+ catalogObj,
+ catalogsMaintenanceConfig.catalogRolesRetain().orElse(DEFAULT_CATALOG_ROLES_RETAIN),
+ CatalogRolesObj.class,
+ CatalogRolesObj::nameToObjRef,
+ collector,
+ catalogRolesObj -> collector.indexRetain(catalogRolesObj.stableIdToName()));
+
+ LOGGER.info(
+ "Identifying catalog state for catalog '{}' ({})...",
+ catalogObj.name(),
+ catalogObj.stableId());
+ ignoreReferenceNotFound(
+ () -> {
+ var catalogStateRefName =
+ format(CATALOG_STATE_REF_NAME_PATTERN, catalogObj.stableId());
+ var catalogStateContinue =
+ new CelReferenceContinuePredicate<CatalogStateObj>(
+ catalogStateRefName,
+ persistence,
+ catalogsMaintenanceConfig
+ .catalogStateRetain()
+ .orElse(DEFAULT_CATALOG_STATE_RETAIN));
+ collector.refRetainIndexToSingleObj(
+ catalogStateRefName,
+ CatalogStateObj.class,
+ catalogStateContinue,
+ CatalogStateObj::nameToObjRef,
+ new RetainedCollector.ProgressListener<>() {
+ public static final long PROGRESS_LOG_INTERVAL_MS = 2_000L;
+ private long commit;
+ private long nextLog =
+ monotonicClock.currentTimeMillis() + PROGRESS_LOG_INTERVAL_MS;
+
+ @Override
+ public void onCommit(CatalogStateObj catalogStateObj, long commit) {
+ collector.indexRetain(catalogStateObj.stableIdToName());
+ catalogStateObj.locations().ifPresent(collector::indexRetain);
+ catalogStateObj.changes().ifPresent(collector::indexRetain);
+ this.commit = commit;
+ }
+
+ @Override
+ public void onIndexEntry(long inCommit, long total) {
+ var now = monotonicClock.currentTimeMillis();
+ if (now >= nextLog) {
+ LOGGER.info(
+ "... {} total index entries processed so far, at commit {}",
+ total,
+ commit);
+ nextLog = now + PROGRESS_LOG_INTERVAL_MS;
+ }
+ }
+ },
+ x -> {});
Review Comment:
can we have meaningful name what does `x` denotes ?
##########
persistence/nosql/persistence/metastore-maintenance/src/main/java/org/apache/polaris/persistence/nosql/metastore/maintenance/CatalogsMaintenanceConfig.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.metastore.maintenance;
+
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import io.smallrye.config.ConfigMapping;
+import io.smallrye.config.WithDefault;
+import java.util.Optional;
+import org.apache.polaris.immutables.PolarisImmutable;
+
+/**
+ * Polaris stores a history of changes per kind of object (principals, principal roles, grants,
+ * immediate tasks, catalog roles and catalog state).
Review Comment:
This is not true for all persistence in Apache Polaris.
```suggestion
* No SQL persistence implementation of Polaris stores a history of changes per kind of object (principals, principal roles, grants,
* immediate tasks, catalog roles and catalog state).
```
##########
persistence/nosql/persistence/metastore-maintenance/src/main/java/org/apache/polaris/persistence/nosql/metastore/maintenance/CatalogsMaintenanceConfig.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.metastore.maintenance;
+
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import io.smallrye.config.ConfigMapping;
+import io.smallrye.config.WithDefault;
+import java.util.Optional;
+import org.apache.polaris.immutables.PolarisImmutable;
+
+/**
+ * Polaris stores a history of changes per kind of object (principals, principal roles, grants,
+ * immediate tasks, catalog roles and catalog state).
+ *
+ * <p>The rules are defined using a <a href="https://github.com/projectnessie/cel-java/">CEL
Review Comment:
why are we using this library to define the rules, have we considered alternatives ? what kind of rules is required ?
##########
persistence/nosql/persistence/metastore-maintenance/src/main/java/org/apache/polaris/persistence/nosql/metastore/maintenance/CatalogsMaintenanceConfig.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.metastore.maintenance;
+
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import io.smallrye.config.ConfigMapping;
+import io.smallrye.config.WithDefault;
+import java.util.Optional;
+import org.apache.polaris.immutables.PolarisImmutable;
+
+/**
+ * Polaris stores a history of changes per kind of object (principals, principal roles, grants,
+ * immediate tasks, catalog roles and catalog state).
+ *
+ * <p>The rules are defined using a <a href="https://github.com/projectnessie/cel-java/">CEL
+ * script</a>. The default rules for all kinds of objects are to retain the history for 3 days, for
+ * the catalog state for 30 days.
+ *
+ * <p>The scripts have access to the following declared values:
+ *
+ * <ul>
+ * <li>{@code ref} (string) name of the reference
+ * <li>{@code commits} (64-bit int) number of the currently processed commit, starting at {@code
+ * 1}
+ * <li>{@code ageDays} (64-bit int) age of currently processed commit in days
+ * <li>{@code ageHours} (64-bit int) age of currently processed commit in hours
+ * <li>{@code ageMinutes} (64-bit int) age of currently processed commit in minutes
+ * </ul>
+ *
+ * <p>Scripts <em>must</em> return a {@code boolean} yielding whether the commit shall be retained.
+ * Note that maintenance-service implementations can keep the first not-to-be-retained commit.
+ *
+ * <p>Example scripts
+ *
+ * <ul>
+ * <li>{@code ageDays < 30 || commits <= 10} retains the reference history with at least 10
+ * commits and commits that are younger than 30 days
+ * <li>{@code true} retains the whole reference history
+ * <li>{@code false} retains the most recent commit
+ * </ul>
+ */
+@ConfigMapping(prefix = "polaris.persistence.maintenance.catalog")
Review Comment:
```suggestion
@ConfigMapping(prefix = "polaris.persistence.nosql.maintenance.catalog")
```
this is not persistence implementation agnostic
prev discussion : https://lists.apache.org/thread/vmdpb45j1nmmlhswz7fm87gxgoldmr28
similar callout: https://github.com/apache/polaris/pull/3135#discussion_r2558515915
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3268:
URL: https://github.com/apache/polaris/pull/3268#discussion_r2670145069
##########
persistence/nosql/persistence/metastore-maintenance/src/main/java/org/apache/polaris/persistence/nosql/metastore/maintenance/CatalogsMaintenanceConfig.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.metastore.maintenance;
+
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import io.smallrye.config.ConfigMapping;
+import io.smallrye.config.WithDefault;
+import java.util.Optional;
+import org.apache.polaris.immutables.PolarisImmutable;
+
+/**
+ * Polaris stores a history of changes per kind of object (principals, principal roles, grants,
+ * immediate tasks, catalog roles and catalog state).
+ *
+ * <p>The rules are defined using a <a href="https://github.com/projectnessie/cel-java/">CEL
+ * script</a>. The default rules for all kinds of objects are to retain the history for 3 days, for
+ * the catalog state for 30 days.
+ *
+ * <p>The scripts have access to the following declared values:
+ *
+ * <ul>
+ * <li>{@code ref} (string) name of the reference
+ * <li>{@code commits} (64-bit int) number of the currently processed commit, starting at {@code
+ * 1}
+ * <li>{@code ageDays} (64-bit int) age of currently processed commit in days
+ * <li>{@code ageHours} (64-bit int) age of currently processed commit in hours
+ * <li>{@code ageMinutes} (64-bit int) age of currently processed commit in minutes
+ * </ul>
+ *
+ * <p>Scripts <em>must</em> return a {@code boolean} yielding whether the commit shall be retained.
+ * Note that maintenance-service implementations can keep the first not-to-be-retained commit.
+ *
+ * <p>Example scripts
+ *
+ * <ul>
+ * <li>{@code ageDays < 30 || commits <= 10} retains the reference history with at least 10
+ * commits and commits that are younger than 30 days
+ * <li>{@code true} retains the whole reference history
+ * <li>{@code false} retains the most recent commit
+ * </ul>
+ */
+@ConfigMapping(prefix = "polaris.persistence.maintenance.catalog")
+@JsonSerialize(as = ImmutableBuildableCatalogsMaintenanceConfig.class)
+@JsonDeserialize(as = ImmutableBuildableCatalogsMaintenanceConfig.class)
+public interface CatalogsMaintenanceConfig {
+
+ String DEFAULT_PRINCIPALS_RETAIN = "false";
+ String DEFAULT_PRINCIPAL_ROLES_RETAIN = "false";
+ String DEFAULT_GRANTS_RETAIN = "false";
+ String DEFAULT_IMMEDIATE_TASKS_RETAIN = "false";
+ String DEFAULT_CATALOGS_HISTORY_RETAIN = "false";
+ String DEFAULT_CATALOG_ROLES_RETAIN = "false";
+ String DEFAULT_CATALOG_POLICIES_RETAIN = "ageDays < 30 || commits <= 1";
+ String DEFAULT_CATALOG_STATE_RETAIN = "ageDays < 30 || commits <= 1";
Review Comment:
I do not think this is the "only thing". This rule is just a fairly safe common sense default.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on PR #3268:
URL: https://github.com/apache/polaris/pull/3268#issuecomment-3731173052
About the concern or about the revert ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3268:
URL: https://github.com/apache/polaris/pull/3268#discussion_r2670139185
##########
persistence/nosql/persistence/metastore-maintenance/src/main/java/org/apache/polaris/persistence/nosql/metastore/maintenance/CatalogsMaintenanceConfig.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.metastore.maintenance;
+
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import io.smallrye.config.ConfigMapping;
+import io.smallrye.config.WithDefault;
+import java.util.Optional;
+import org.apache.polaris.immutables.PolarisImmutable;
+
+/**
+ * Polaris stores a history of changes per kind of object (principals, principal roles, grants,
+ * immediate tasks, catalog roles and catalog state).
+ *
+ * <p>The rules are defined using a <a href="https://github.com/projectnessie/cel-java/">CEL
Review Comment:
`projectnessie/cel-java` is an implementation of the CEL spec in java. Unlike Google's impl. it also works with Jackson.
IMHO, CEL is simple and performant, while wielding a decent expressive power. I think it is a reasonable initial option for the rules language. I suggest merging this PR witl CEL for the sake of making progress. It is certainly open for discussion if alternatives are proposed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3268:
URL: https://github.com/apache/polaris/pull/3268#discussion_r2670139185
##########
persistence/nosql/persistence/metastore-maintenance/src/main/java/org/apache/polaris/persistence/nosql/metastore/maintenance/CatalogsMaintenanceConfig.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.metastore.maintenance;
+
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import io.smallrye.config.ConfigMapping;
+import io.smallrye.config.WithDefault;
+import java.util.Optional;
+import org.apache.polaris.immutables.PolarisImmutable;
+
+/**
+ * Polaris stores a history of changes per kind of object (principals, principal roles, grants,
+ * immediate tasks, catalog roles and catalog state).
+ *
+ * <p>The rules are defined using a <a href="https://github.com/projectnessie/cel-java/">CEL
Review Comment:
`projectnessie/cel-java` is an implementation of the CEL spec in java. Unlike Google's impl. it also works with Jackson.
IMHO, CEL is simple and performant, while wielding a decent expressive power. I think it is a reasonable initial option for the rules language. It is certainly open for discussion if alternatives are proposed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on PR #3268:
URL: https://github.com/apache/polaris/pull/3268#issuecomment-3730739628
@dimas-b can you please explain how were the feedbacks mentioned here addressed : https://github.com/apache/polaris/pull/3268#pullrequestreview-3576273215
I am not sure until we have resolved the feedbacks how can we move forward, if the feedbacks is not addressed for 3 week hence the pr is opened for 3 weeks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3268:
URL: https://github.com/apache/polaris/pull/3268#issuecomment-3731138172
@singhpk234 : The config concern was in fact addressed. Config names are like `polaris.persistence.nosql.maintenance`. If something was unintentionally missed in config names, let's fix-forward on `main`.
Re: the location of this code - I commented above. I do believe it explained the intention. A possible user-side tool is provided in #3353
Re: CEL, please see my comment [above](https://github.com/apache/polaris/pull/3268#discussion_r2670139185). This is why Nessie's CEL impl. is convenient. Please note that `projectnessie/cel-java` is a pure CEL impl. It has nothing to do with the Nessie Catalog.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3268:
URL: https://github.com/apache/polaris/pull/3268#issuecomment-3661255125
> Seeing the end-to-end admin flow would also help contextualize the split of "server-side" and "client-side" code related to this maintenance, that @dimas-b talked about
Hypothetically (ATM), the Admin tool could involve the maintenance code to perform its job.
Other implementations are possible too. It's an open-ended design where downstream projects are free to implement their own triggering mechanisms. I imagine k8s jobs is one of possible options.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #3268:
URL: https://github.com/apache/polaris/pull/3268#discussion_r2668927067
##########
persistence/nosql/persistence/metastore-maintenance/src/main/java/org/apache/polaris/persistence/nosql/metastore/maintenance/CatalogsMaintenanceConfig.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.metastore.maintenance;
+
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import io.smallrye.config.ConfigMapping;
+import io.smallrye.config.WithDefault;
+import java.util.Optional;
+import org.apache.polaris.immutables.PolarisImmutable;
+
+/**
+ * Polaris stores a history of changes per kind of object (principals, principal roles, grants,
+ * immediate tasks, catalog roles and catalog state).
+ *
+ * <p>The rules are defined using a <a href="https://github.com/projectnessie/cel-java/">CEL
+ * script</a>. The default rules for all kinds of objects are to retain the history for 3 days, for
+ * the catalog state for 30 days.
+ *
+ * <p>The scripts have access to the following declared values:
+ *
+ * <ul>
+ * <li>{@code ref} (string) name of the reference
+ * <li>{@code commits} (64-bit int) number of the currently processed commit, starting at {@code
+ * 1}
+ * <li>{@code ageDays} (64-bit int) age of currently processed commit in days
+ * <li>{@code ageHours} (64-bit int) age of currently processed commit in hours
+ * <li>{@code ageMinutes} (64-bit int) age of currently processed commit in minutes
+ * </ul>
+ *
+ * <p>Scripts <em>must</em> return a {@code boolean} yielding whether the commit shall be retained.
+ * Note that maintenance-service implementations can keep the first not-to-be-retained commit.
+ *
+ * <p>Example scripts
+ *
+ * <ul>
+ * <li>{@code ageDays < 30 || commits <= 10} retains the reference history with at least 10
+ * commits and commits that are younger than 30 days
+ * <li>{@code true} retains the whole reference history
+ * <li>{@code false} retains the most recent commit
+ * </ul>
+ */
+@ConfigMapping(prefix = "polaris.persistence.maintenance.catalog")
+@JsonSerialize(as = ImmutableBuildableCatalogsMaintenanceConfig.class)
+@JsonDeserialize(as = ImmutableBuildableCatalogsMaintenanceConfig.class)
+public interface CatalogsMaintenanceConfig {
+
+ String DEFAULT_PRINCIPALS_RETAIN = "false";
+ String DEFAULT_PRINCIPAL_ROLES_RETAIN = "false";
+ String DEFAULT_GRANTS_RETAIN = "false";
+ String DEFAULT_IMMEDIATE_TASKS_RETAIN = "false";
+ String DEFAULT_CATALOGS_HISTORY_RETAIN = "false";
+ String DEFAULT_CATALOG_ROLES_RETAIN = "false";
+ String DEFAULT_CATALOG_POLICIES_RETAIN = "ageDays < 30 || commits <= 1";
+ String DEFAULT_CATALOG_STATE_RETAIN = "ageDays < 30 || commits <= 1";
Review Comment:
This has nothing to do with "not *yet* committed" data.
This maintenance cleans up stale + unreferenced as well as "unwanted" history. What "unwanted" means is defined by these defaults, which can optionally be [customized by users](https://github.com/apache/polaris/pull/3268/changes/BASE..04742bd59fedcbddaa6419a01914dd07cbf334eb#diff-f96135bd1e400c2ce4aa2de7c173e1e8a7578201818a1be967b014a589a52fb5R39).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3268:
URL: https://github.com/apache/polaris/pull/3268#discussion_r2670139185
##########
persistence/nosql/persistence/metastore-maintenance/src/main/java/org/apache/polaris/persistence/nosql/metastore/maintenance/CatalogsMaintenanceConfig.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.metastore.maintenance;
+
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import io.smallrye.config.ConfigMapping;
+import io.smallrye.config.WithDefault;
+import java.util.Optional;
+import org.apache.polaris.immutables.PolarisImmutable;
+
+/**
+ * Polaris stores a history of changes per kind of object (principals, principal roles, grants,
+ * immediate tasks, catalog roles and catalog state).
+ *
+ * <p>The rules are defined using a <a href="https://github.com/projectnessie/cel-java/">CEL
Review Comment:
`projectnessie/cel-java` is an implementation of the CEL spec in java. Unlike Google's impl. it also works with Jackson.
IMHO, CEL is simple and performant, while wielding a decent expressive power. I think it is a reasonable initial option for the rules language. I suggest merging this PR witl CEL for the sake of making progress. It is certainly open for discussion if alternatives are proposed later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3268:
URL: https://github.com/apache/polaris/pull/3268#issuecomment-3738726648
I mean any/all issue brought up here are probably best discussed on `dev`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3268:
URL: https://github.com/apache/polaris/pull/3268#issuecomment-3731166884
Let's discuss on `dev`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #3268:
URL: https://github.com/apache/polaris/pull/3268#issuecomment-3733501623
> If this is still a concern, let's discuss (as I noted above) and possible use alternatives.
It is a real concern. Before the concern was resolved, I don't think the PR should be merge. It's not acceptable to do so in an ASF project.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on PR #3268:
URL: https://github.com/apache/polaris/pull/3268#issuecomment-3677815549
Thanks all for the helpful reviews!
Documentation of "everything" is definitely planned to be added to the web site once everything is in.
I have renamed the config mapping.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3268:
URL: https://github.com/apache/polaris/pull/3268#discussion_r2623846411
##########
persistence/nosql/persistence/metastore-maintenance/src/main/java/org/apache/polaris/persistence/nosql/metastore/maintenance/CatalogsMaintenanceConfig.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.metastore.maintenance;
+
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import io.smallrye.config.ConfigMapping;
+import io.smallrye.config.WithDefault;
+import java.util.Optional;
+import org.apache.polaris.immutables.PolarisImmutable;
+
+/**
+ * Polaris stores a history of changes per kind of object (principals, principal roles, grants,
+ * immediate tasks, catalog roles and catalog state).
+ *
+ * <p>The rules are defined using a <a href="https://github.com/projectnessie/cel-java/">CEL
+ * script</a>. The default rules for all kinds of objects are to retain the history for 3 days, for
+ * the catalog state for 30 days.
+ *
+ * <p>The scripts have access to the following declared values:
+ *
+ * <ul>
+ * <li>{@code ref} (string) name of the reference
+ * <li>{@code commits} (64-bit int) number of the currently processed commit, starting at {@code
+ * 1}
+ * <li>{@code ageDays} (64-bit int) age of currently processed commit in days
+ * <li>{@code ageHours} (64-bit int) age of currently processed commit in hours
+ * <li>{@code ageMinutes} (64-bit int) age of currently processed commit in minutes
+ * </ul>
+ *
+ * <p>Scripts <em>must</em> return a {@code boolean} yielding whether the commit shall be retained.
+ * Note that maintenance-service implementations can keep the first not-to-be-retained commit.
+ *
+ * <p>Example scripts
+ *
+ * <ul>
+ * <li>{@code ageDays < 30 || commits <= 10} retains the reference history with at least 10
+ * commits and commits that are younger than 30 days
+ * <li>{@code true} retains the whole reference history
+ * <li>{@code false} retains the most recent commit
+ * </ul>
+ */
+@ConfigMapping(prefix = "polaris.persistence.maintenance.catalog")
Review Comment:
Given the discussion in #3135, I think it is reasonable to use the `polaris.persistence.nosql` config prefix in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: Metastore maintenance [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3268:
URL: https://github.com/apache/polaris/pull/3268#issuecomment-3740722178
@flyrain : my points about these items are available in this PR's comments trail.
The need for rules is evident from the related java code.
If you prefer to go deeper into these aspects please bring your concerns to the `dev` ML and I'll be happy to discuss further.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org