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 2026/01/09 10:30:18 UTC
[PR] NoSQL: Add maintenance service to Polaris admin tool [polaris]
snazy opened a new pull request, #3395:
URL: https://github.com/apache/polaris/pull/3395
This change wires the maintenance service up to the Polaris admin tool.
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on PR #3395:
URL: https://github.com/apache/polaris/pull/3395#issuecomment-3836267940
Rebased + force-pushed
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2699490623
##########
runtime/admin/src/main/resources/application.properties:
##########
@@ -40,7 +40,24 @@ quarkus.rds.sync-client.type=apache
# ---- Runtime Configuration ----
# Below are default values for properties that can be changed in runtime.
+# Available types:
+# - in-memory - InMemoryPolarisMetaStoreManagerFactory
+# - in-memory-atomic - InMemoryAtomicOperationMetaStoreManagerFactory
+# - relational-jdbc - JdbcMetaStoreManagerFactory
+# - nosql - NoSQL persistence backend, define the backend type via 'polaris.persistence.nosql.backend'
polaris.persistence.type=relational-jdbc
+# Database backend for 'nosql' persistence-type
+# Available backends:
+# - InMemory - for testing purposes
+# - MongoDb - configure the via the Quarkus extension
+polaris.persistence.nosql.backend=InMemory
+
+## MongoDB version store specific configuration
Review Comment:
Same comments request here as on the service pr, These properties should probably be commented out by default since they aren't used
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2834425861
##########
site/content/in-dev/unreleased/admin-tool.md:
##########
@@ -208,3 +209,85 @@ docker run --rm -it \
```
Again, the Polaris Admin Tool must be executed with appropriate configuration to connect to the same database used by the Polaris server. The configuration can be done via environment variables (as above) or system properties.
+
+## NoSQL Specific Operations
+
+The `nosql` admin tool command is used to perform NoSQL metastore specific operations.
+```
+Usage: polaris-admin-tool.jar nosql [-hV] [COMMAND]
+Polaris NoSQL persistence.
+ -h, --help Show this help message and exit.
+ -V, --version Print version information and exit.
+Commands:
+ maintenance-info Polaris NoSQL persistence maintenance information.
+ maintenance-log Show Polaris persistence maintenance log.
+ maintenance-run Run Polaris persistence maintenance.
+```
+
+### Running NoSQL maintenance
+
+NoSQL maintenance is run using the `nosql maintenance-run` command.
+Maintenance covers the deletion of stale database entries.
+
+{{< alert note >}}
+It is recommended to run the `nosql maintenance-run` command regulary, for example, once per day.
+{{< /alert >}}
+
+The output shows a bunch of configuration options, most of which are automatically determined by the tool.
+
+```
+Maintenance configuration:
+ created-at grace time: PT3H
+ delete batch size: 10
+ retained runs: 50
+ expected object count: 100000
+ expected reference count: 100
+ last-run multiplier: 1.100000
+ initialized FPP: 0.000010
+ expected FPP: 0.000050
+ reference scan rate limit / sec: (unlimited)
+ object scan rate limit / sec: (unlimited)
+```
+
+The remaining output is a detailed report of the maintenance run.
+
+The implementation of NoSQL maintenance is self-tuning and does usually not require additional configuration.
+
+If you encounter that the maintenance run operation puts too much load on the database,
+you can adjust scan rates to reduce the load.
+The option for the reference and object scan rates are
+`polaris.persistence.nosql.maintenance.object-scan-rate-limit-per-second` and
+`polaris.persistence.nosql.maintenance.reference-scan-rate-limit-per-second`, both default to 'unlimited.'
+It is safe to let the maintenance run for a long time, there is no runtime limit.
+Alternatively, you can run maintenance at high or the default unlimited scan rate during off-peak hours.
+
+{{< alert note >}}
+The `nosql maintenance-run` command may emit a warning that not all purgable database entried have been deleted.
Review Comment:
typo: entried -> entries
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3395:
URL: https://github.com/apache/polaris/pull/3395#issuecomment-3935332350
@dennishuo : I updated the PR discription... Is it better? :)
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2759733345
##########
runtime/admin/src/main/resources/application.properties:
##########
@@ -50,10 +67,13 @@ quarkus.datasource.devservices.enabled=false
quarkus.arc.ignored-split-packages=\
org.apache.polaris.admintool.config,\
+ org.apache.polaris.admintool.maintenance,\
Review Comment:
I'll deal with the other warnings on `main` 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: Add maintenance service to Polaris admin tool [polaris]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2699775983
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/maintenance/NoSqlMaintenanceCommand.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * 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.admintool.maintenance;
+
+import picocli.CommandLine;
+
+@CommandLine.Command(
+ name = "maintenance",
+ subcommands = {
+ NoSqlMaintenanceLogCommand.class,
+ NoSqlMaintenanceRunCommand.class,
+ },
+ mixinStandardHelpOptions = true,
+ description = "Polaris persistence maintenance.")
+public class NoSqlMaintenanceCommand extends BaseMaintenanceCommand {
+
+ @Override
+ public Integer call() {
+ var out = spec.commandLine().getOut();
+
+ out.println("Polaris NoSql persistence maintenance has multiple subcommands,");
+ out.println("use the 'help maintenance' command.");
+ out.println();
+
+ checkInMemory();
+
+ out.println();
+ out.println("Information: selected NoSql persistence backend: " + backend.type());
Review Comment:
In the current formulation this isn't correct since you can call this with any backend right?
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2718362690
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/nosql/NoSqlCommand.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.admintool.nosql;
+
+import jakarta.inject.Inject;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceInfoCommand;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceLogCommand;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceRunCommand;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import picocli.CommandLine;
+
+@CommandLine.Command(
+ name = "nosql",
Review Comment:
+1
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2699730423
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/maintenance/NoSqlMaintenanceCommand.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * 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.admintool.maintenance;
+
+import picocli.CommandLine;
+
+@CommandLine.Command(
+ name = "maintenance",
+ subcommands = {
+ NoSqlMaintenanceLogCommand.class,
+ NoSqlMaintenanceRunCommand.class,
+ },
+ mixinStandardHelpOptions = true,
+ description = "Polaris persistence maintenance.")
+public class NoSqlMaintenanceCommand extends BaseMaintenanceCommand {
+
+ @Override
+ public Integer call() {
+ var out = spec.commandLine().getOut();
+
+ out.println("Polaris NoSql persistence maintenance has multiple subcommands,");
+ out.println("use the 'help maintenance' command.");
+ out.println();
+
+ checkInMemory();
Review Comment:
Shouldn't we be failing for everything that isn't NoSql not just InMemory?
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy merged PR #3395:
URL: https://github.com/apache/polaris/pull/3395
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2748573061
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/nosql/maintenance/NoSqlMaintenanceRunCommand.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.admintool.maintenance;
+
+import jakarta.inject.Inject;
+import org.apache.polaris.persistence.nosql.maintenance.api.MaintenanceService;
+import picocli.CommandLine;
+
+@CommandLine.Command(
+ name = "run",
+ mixinStandardHelpOptions = true,
+ description = {"Run Polaris persistence maintenance."})
Review Comment:
... and similar description changes in the other new commands?
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2699783382
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/maintenance/BaseMaintenanceCommand.java:
##########
@@ -0,0 +1,190 @@
+/*
+ * 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.admintool.maintenance;
+
+import static org.apache.polaris.persistence.nosql.maintenance.api.MaintenanceConfig.DEFAULT_COUNT_FROM_LAST_RUN_MULTIPLIER;
+import static org.apache.polaris.persistence.nosql.maintenance.api.MaintenanceConfig.DEFAULT_CREATED_AT_GRACE_TIME;
+import static org.apache.polaris.persistence.nosql.maintenance.api.MaintenanceConfig.DEFAULT_DELETE_BATCH_SIZE;
+import static org.apache.polaris.persistence.nosql.maintenance.api.MaintenanceConfig.DEFAULT_EXPECTED_OBJ_COUNT;
+import static org.apache.polaris.persistence.nosql.maintenance.api.MaintenanceConfig.DEFAULT_EXPECTED_REFERENCE_COUNT;
+import static org.apache.polaris.persistence.nosql.maintenance.api.MaintenanceConfig.DEFAULT_INITIALIZED_FPP;
+import static org.apache.polaris.persistence.nosql.maintenance.api.MaintenanceConfig.DEFAULT_MAX_ACCEPTABLE_FPP;
+import static org.apache.polaris.persistence.nosql.maintenance.api.MaintenanceConfig.DEFAULT_RETAINED_RUNS;
+
+import jakarta.inject.Inject;
+import java.io.PrintWriter;
+import java.time.Instant;
+import org.apache.polaris.admintool.BaseCommand;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.obj.ObjTypes;
+import org.apache.polaris.persistence.nosql.maintenance.api.MaintenanceConfig;
+import org.apache.polaris.persistence.nosql.maintenance.api.MaintenanceRunInformation;
+import org.apache.polaris.persistence.nosql.maintenance.api.MaintenanceRunInformation.MaintenanceStats;
+import org.apache.polaris.persistence.nosql.maintenance.api.MaintenanceRunSpec;
+import org.apache.polaris.persistence.nosql.maintenance.api.MaintenanceService;
+
+@SuppressWarnings("CdiInjectionPointsInspection")
+public abstract class BaseMaintenanceCommand extends BaseCommand {
+ @Inject protected MaintenanceService maintenanceService;
+ @Inject protected Backend backend;
+ @Inject protected MaintenanceConfig maintenanceConfig;
+
+ protected void checkInMemory() {
+ if ("InMemory".equals(backend.type())) {
Review Comment:
Do we need a check here for the persistence not being NoSQL?
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #3395:
URL: https://github.com/apache/polaris/pull/3395#issuecomment-3850720388
Sorry for the late reply, I have posted the concern in the dev ML https://lists.apache.org/thread/93nvjwwbvv77fz16rsdcwsm1m8nc5trt
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2748581434
##########
runtime/admin/src/main/resources/application.properties:
##########
@@ -50,10 +67,13 @@ quarkus.datasource.devservices.enabled=false
quarkus.arc.ignored-split-packages=\
org.apache.polaris.admintool.config,\
+ org.apache.polaris.admintool.maintenance,\
Review Comment:
Package `org.apache.polaris.admintool.nosql.maintenance` only exists in one module, that is in `runtime/admin`... Is this config required anymore? 🤔
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2748223158
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/nosql/maintenance/NoSqlMaintenanceRunCommand.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.admintool.maintenance;
+
+import jakarta.inject.Inject;
+import org.apache.polaris.persistence.nosql.maintenance.api.MaintenanceService;
+import picocli.CommandLine;
+
+@CommandLine.Command(
+ name = "run",
+ mixinStandardHelpOptions = true,
+ description = {"Run Polaris persistence maintenance."})
Review Comment:
@snazy : WDYT?
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2756574906
##########
runtime/admin/src/main/resources/application.properties:
##########
@@ -50,10 +69,13 @@ quarkus.datasource.devservices.enabled=false
quarkus.arc.ignored-split-packages=\
org.apache.polaris.admintool.config,\
+ org.apache.polaris.admintool.maintenance,\
org.apache.polaris.admintool
## Quarkus required setting for third party indexing
# fixed at build-time
+quarkus.index-dependency.agrona.group-id=org.agrona
+quarkus.index-dependency.agrona.artifact-id=agrona
Review Comment:
I re-ran my local build after the recent rebase _without_ this property and I did not see any build warnings about Agrona being not index.
@snazy : WDYT about removing this property?
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2756574906
##########
runtime/admin/src/main/resources/application.properties:
##########
@@ -50,10 +69,13 @@ quarkus.datasource.devservices.enabled=false
quarkus.arc.ignored-split-packages=\
org.apache.polaris.admintool.config,\
+ org.apache.polaris.admintool.maintenance,\
org.apache.polaris.admintool
## Quarkus required setting for third party indexing
# fixed at build-time
+quarkus.index-dependency.agrona.group-id=org.agrona
+quarkus.index-dependency.agrona.artifact-id=agrona
Review Comment:
I re-ran my local build after the recent rebase _without_ this property and I did not see any build warnings about Agrona being unindexed.
@snazy : WDYT about removing this property?
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2831854233
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/nosql/NoSqlCommand.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.admintool.nosql;
+
+import jakarta.inject.Inject;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceInfoCommand;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceLogCommand;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceRunCommand;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import picocli.CommandLine;
+
+@CommandLine.Command(
+ name = "nosql",
+ subcommands = {
+ NoSqlMaintenanceInfoCommand.class,
+ NoSqlMaintenanceLogCommand.class,
+ NoSqlMaintenanceRunCommand.class,
+ },
+ mixinStandardHelpOptions = true,
+ description = "Polaris NoSQL persistence.")
+public class NoSqlCommand extends BaseNoSqlCommand {
Review Comment:
@dimas-b is right.
Java type hierarchy is orthogonal to picocli commands / annotations.
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on PR #3395:
URL: https://github.com/apache/polaris/pull/3395#issuecomment-3935983695
Added some docs for the web site.
The reference there to the "Configuration Reference" should be updated either in #3838 or here (whichever gets merged first.)
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2748562827
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/nosql/maintenance/NoSqlMaintenanceInfoCommand.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * 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.admintool.nosql.maintenance;
+
+import picocli.CommandLine;
+
+@CommandLine.Command(
+ name = "maintenance-info",
Review Comment:
nit: Maybe add `maintenance` as another sub-command group?
Current: `java -jar ... nosql maintenance-info`
Proposed: `java -jar ... nosql maintenance info`
I hope the latter in nicer in terms of UX.
##########
runtime/admin/src/main/resources/application.properties:
##########
@@ -40,7 +40,24 @@ quarkus.rds.sync-client.type=apache
# ---- Runtime Configuration ----
# Below are default values for properties that can be changed in runtime.
+# Available types:
+# - in-memory - InMemoryPolarisMetaStoreManagerFactory
+# - in-memory-atomic - InMemoryAtomicOperationMetaStoreManagerFactory
+# - relational-jdbc - JdbcMetaStoreManagerFactory
+# - nosql - NoSQL persistence backend, define the backend type via 'polaris.persistence.nosql.backend'
polaris.persistence.type=relational-jdbc
+# Database backend for 'nosql' persistence-type
+# Available backends:
+# - InMemory - for testing purposes
+# - MongoDb - configure the via the Quarkus extension
+polaris.persistence.nosql.backend=InMemory
+
+## MongoDB version store specific configuration
+quarkus.mongodb.database=polaris
+quarkus.mongodb.metrics.enabled=true
+#quarkus.mongodb.connection-string=mongodb://localhost:27017
+quarkus.mongodb.devservices.enabled=false
Review Comment:
Per https://quarkus.io/guides/mongodb-dev-services `quarkus.mongodb.devservices.enabled` is a build-time config. Please move to the `Build-time configuration` section (lines 20-38) for the sake of consistency.
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/nosql/NoSqlCommand.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.admintool.nosql;
+
+import jakarta.inject.Inject;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceInfoCommand;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceLogCommand;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceRunCommand;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import picocli.CommandLine;
+
+@CommandLine.Command(
+ name = "nosql",
+ subcommands = {
+ NoSqlMaintenanceInfoCommand.class,
+ NoSqlMaintenanceLogCommand.class,
+ NoSqlMaintenanceRunCommand.class,
+ },
+ mixinStandardHelpOptions = true,
+ description = "Polaris NoSQL persistence.")
Review Comment:
nit: suggestion: `Sub-commands specific to NoSQL persistence.`
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3395:
URL: https://github.com/apache/polaris/pull/3395#issuecomment-3849006014
@flyrain : Could you review again, please? There were code updates and fresh discussion points after your previous -1 review.
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dennishuo (via GitHub)" <gi...@apache.org>.
dennishuo commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2831026611
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/nosql/NoSqlCommand.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.admintool.nosql;
+
+import jakarta.inject.Inject;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceInfoCommand;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceLogCommand;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceRunCommand;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import picocli.CommandLine;
+
+@CommandLine.Command(
+ name = "nosql",
+ subcommands = {
+ NoSqlMaintenanceInfoCommand.class,
+ NoSqlMaintenanceLogCommand.class,
+ NoSqlMaintenanceRunCommand.class,
+ },
+ mixinStandardHelpOptions = true,
+ description = "Polaris NoSQL persistence.")
+public class NoSqlCommand extends BaseNoSqlCommand {
Review Comment:
Reading through the code to answer my own question :)
I guess the `subcommands` tuple basically "intercepts" the other command classes rather than requiring those other command classes to know anything about being nested under a parent command?
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2831107488
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/nosql/NoSqlCommand.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.admintool.nosql;
+
+import jakarta.inject.Inject;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceInfoCommand;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceLogCommand;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceRunCommand;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import picocli.CommandLine;
+
+@CommandLine.Command(
+ name = "nosql",
+ subcommands = {
+ NoSqlMaintenanceInfoCommand.class,
+ NoSqlMaintenanceLogCommand.class,
+ NoSqlMaintenanceRunCommand.class,
+ },
+ mixinStandardHelpOptions = true,
+ description = "Polaris NoSQL persistence.")
+public class NoSqlCommand extends BaseNoSqlCommand {
Review Comment:
AFAIK, command nesting is derived from annotations. Implementation classes are free to extend other classes just to share code.
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2835050431
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/nosql/NoSqlCommand.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.admintool.nosql;
+
+import jakarta.inject.Inject;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceInfoCommand;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceLogCommand;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceRunCommand;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import picocli.CommandLine;
+
+@CommandLine.Command(
+ name = "nosql",
+ subcommands = {
+ NoSqlMaintenanceInfoCommand.class,
+ NoSqlMaintenanceLogCommand.class,
+ NoSqlMaintenanceRunCommand.class,
+ },
+ mixinStandardHelpOptions = true,
+ description = "Polaris NoSQL persistence.")
Review Comment:
I took the liberty to push this update from my side. @snazy : I hope you do not mind.
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dennishuo (via GitHub)" <gi...@apache.org>.
dennishuo commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2830981605
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/nosql/NoSqlCommand.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.admintool.nosql;
+
+import jakarta.inject.Inject;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceInfoCommand;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceLogCommand;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceRunCommand;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import picocli.CommandLine;
+
+@CommandLine.Command(
+ name = "nosql",
+ subcommands = {
+ NoSqlMaintenanceInfoCommand.class,
+ NoSqlMaintenanceLogCommand.class,
+ NoSqlMaintenanceRunCommand.class,
+ },
+ mixinStandardHelpOptions = true,
+ description = "Polaris NoSQL persistence.")
+public class NoSqlCommand extends BaseNoSqlCommand {
Review Comment:
Just a question for my own understanding, since I'm not too familiar with how the command hierarchy works - how does this "sibling" command end up causing the nesting discussed on the dev list? It looks like the hierarchy is:
NoSqlMaintenanceRunCommand extends BaseNoSqlMaintenanceCommand
BaseNoSqlMaintenanceCommand extends BaseNoSqlCommand
BaseNoSqlCommand extends BaseCommand
While
NoSqlCommand extends BaseNoSqlCommand
Thus appears to be a "sibling" of `BaseNoSqlMaintenanceCommand ` rather than being a "parent".
Some documentation on the expected full example commands would also be helpful to reason about this; could we add some to the PR description and/or readme/md docs?
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/nosql/NoSqlCommand.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.admintool.nosql;
+
+import jakarta.inject.Inject;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceInfoCommand;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceLogCommand;
+import org.apache.polaris.admintool.nosql.maintenance.NoSqlMaintenanceRunCommand;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import picocli.CommandLine;
+
+@CommandLine.Command(
+ name = "nosql",
+ subcommands = {
+ NoSqlMaintenanceInfoCommand.class,
+ NoSqlMaintenanceLogCommand.class,
+ NoSqlMaintenanceRunCommand.class,
+ },
+ mixinStandardHelpOptions = true,
+ description = "Polaris NoSQL persistence.")
Review Comment:
+1 to this rewording
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2699734506
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/maintenance/NoSqlMaintenanceCommand.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * 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.admintool.maintenance;
+
+import picocli.CommandLine;
+
+@CommandLine.Command(
+ name = "maintenance",
+ subcommands = {
+ NoSqlMaintenanceLogCommand.class,
+ NoSqlMaintenanceRunCommand.class,
+ },
+ mixinStandardHelpOptions = true,
+ description = "Polaris persistence maintenance.")
Review Comment:
I think we also probably shouldn't call it persistence maintenance unless we plan on having maintenance for other backends?
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3395:
URL: https://github.com/apache/polaris/pull/3395#issuecomment-3937072852
CI failed because of: https://lists.apache.org/thread/ln4smllk1j19on3qqq840qldf9yw19hq
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3395:
URL: https://github.com/apache/polaris/pull/3395#issuecomment-3826916806
Summarizing the [dev discussion](https://lists.apache.org/thread/znw87l7r1gh3l09pc0hxhj5gyl8vp28d).
- It looks like there is general consensus on adding NoSQL maintenance commands to the Admin Tool, scoped under a `nosql` top-level command group.
- CEL appears to be regarded as excessive in terms of the flexibility and code maintenance it brings for the use cases it enabled. I propose merging this PR as is (since it does not touch CEL directly) and converting NoSQL maintenance rules from CEL to java code + config in a follow up PR. I'll open a GH issue for this if other people of ok with this approach.
- Further refactorings / renames are certainly possible and I invite ideas / proposals to be expressed on the `dev` thread (link above) or as GH issues.
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3395:
URL: https://github.com/apache/polaris/pull/3395#issuecomment-3826925780
@flyrain : Would you please reconsider your -1 on this PR given the above discussion?
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3395:
URL: https://github.com/apache/polaris/pull/3395#issuecomment-3751988530
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3395:
URL: https://github.com/apache/polaris/pull/3395#issuecomment-3739090270
@flyrain :
> I do not think the NoSQL maintenance tool belongs in the runtime admin module. [...]
NoSQL persistence (by design) needs periodic maintenance by admin users. Not providing a tool for this will mean that no Apache Polaris user will be able to use NoSQL persistence efficiently.
As to the location of the tool, Admin CLI is a natural place as it hosts code for `bootstrap` and other administrative / setup tasks.
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2748579756
##########
runtime/admin/src/main/resources/application.properties:
##########
@@ -50,10 +67,13 @@ quarkus.datasource.devservices.enabled=false
quarkus.arc.ignored-split-packages=\
org.apache.polaris.admintool.config,\
+ org.apache.polaris.admintool.maintenance,\
Review Comment:
Note: this package no longer exists.
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3395:
URL: https://github.com/apache/polaris/pull/3395#issuecomment-3836139195
@snazy : could you rebased this PR? I suspect some of the config concerns are related to the other NoSQL modules merged to `main` but not preset under this PR's git history :thinking:
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2748585794
##########
runtime/admin/src/main/resources/application.properties:
##########
@@ -50,10 +69,13 @@ quarkus.datasource.devservices.enabled=false
quarkus.arc.ignored-split-packages=\
org.apache.polaris.admintool.config,\
+ org.apache.polaris.admintool.maintenance,\
org.apache.polaris.admintool
## Quarkus required setting for third party indexing
# fixed at build-time
+quarkus.index-dependency.agrona.group-id=org.agrona
+quarkus.index-dependency.agrona.artifact-id=agrona
Review Comment:
I tried building locally without these settings (with the [workaround](https://github.com/apache/polaris/issues/3467#issuecomment-3817989427) to get build warnings), but `agrona` was not flagged as needing an index... Is this setting required for the admin tool?
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2756649113
##########
runtime/admin/src/main/resources/application.properties:
##########
@@ -50,10 +67,13 @@ quarkus.datasource.devservices.enabled=false
quarkus.arc.ignored-split-packages=\
org.apache.polaris.admintool.config,\
+ org.apache.polaris.admintool.maintenance,\
Review Comment:
Actually, I see different split package warnings in my build:
```
2026-02-03T00:51:08.714249614Z build-15 WARN Detected a split package usage which is considered a bad practice and should be avoided. Following packages were detected in multiple archives:
- "org.apache.polaris.service.catalog.api" found in [org.apache.polaris:polaris-api-catalog-service:1.4.0-incubating-SNAPSHOT, org.apache.polaris:polaris-api-iceberg-service:1.4.0-incubating-SNAPSHOT]
- "org.apache.polaris.service.catalog.api.impl" found in [org.apache.polaris:polaris-api-catalog-service:1.4.0-incubating-SNAPSHOT, org.apache.polaris:polaris-api-iceberg-service:1.4.0-incubating-SNAPSHOT]
- "org.apache.polaris.service.types" found in [org.apache.polaris:polaris-api-catalog-service:1.4.0-incubating-SNAPSHOT, org.apache.polaris:polaris-api-iceberg-service:1.4.0-incubating-SNAPSHOT]
```
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2759699124
##########
runtime/admin/src/main/resources/application.properties:
##########
@@ -50,6 +69,7 @@ quarkus.datasource.devservices.enabled=false
quarkus.arc.ignored-split-packages=\
org.apache.polaris.admintool.config,\
+ org.apache.polaris.admintool.maintenance,\
Review Comment:
```suggestion
```
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3395:
URL: https://github.com/apache/polaris/pull/3395#issuecomment-3946613272
@flyrain : I opened #3867 to allow users flexibility over dependencies in the Admin CLI. I intend to work on that follow-up issue immediately after merging this PR.
Do you think we can go ahead with this plan? Thx!
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3395:
URL: https://github.com/apache/polaris/pull/3395#issuecomment-3931329828
@dennishuo : Here's the issue for CEL follow-up: #3847
Re: Admin CLI docs, would you mind if they were added after this PR? The latest docs pages are still useful even if they do not make it to the same release as the binaries.
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3395:
URL: https://github.com/apache/polaris/pull/3395#issuecomment-3826938164
@RussellSpitzer : just FYI: I re-requested your review my accident :sweat_smile: but fresh comments are welcome, of course :slightly_smiling_face:
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on PR #3395:
URL: https://github.com/apache/polaris/pull/3395#issuecomment-3761648981
As discussed on the mailing list, I think this is fine to add into the Admin tool but the current PR needs a bit of work to make it clear to end users that it is just for NoSQL.
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2759731919
##########
runtime/admin/src/main/resources/application.properties:
##########
@@ -50,10 +67,13 @@ quarkus.datasource.devservices.enabled=false
quarkus.arc.ignored-split-packages=\
org.apache.polaris.admintool.config,\
+ org.apache.polaris.admintool.maintenance,\
Review Comment:
I've added a commit to remove this prop.
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2677045033
##########
runtime/admin/src/main/resources/application.properties:
##########
@@ -40,7 +40,24 @@ quarkus.rds.sync-client.type=apache
# ---- Runtime Configuration ----
# Below are default values for properties that can be changed in runtime.
+# Available types:
+# - in-memory - InMemoryPolarisMetaStoreManagerFactory
+# - in-memory-atomic - InMemoryAtomicOperationMetaStoreManagerFactory
+# - relational-jdbc - JdbcMetaStoreManagerFactory
+# - nosql - NoSQL persistence backend, define the backend type via 'polaris.persistence.nosql.backend'
polaris.persistence.type=relational-jdbc
+# Database backend for 'nosql' persistence-type
+# Available backends:
+# - InMemory - for testing purposes
+# - MongoDb - configure the via the Quarkus extension
+polaris.persistence.nosql.backend=InMemory
+
+## MongoDB version store specific configuration
+quarkus.mongodb.database=polaris
+quarkus.mongodb.metrics.enabled=true
+#quarkus.mongodb.connection-string=mongodb://localhost:27017
+quarkus.mongodb.devservices.enabled=false
Review Comment:
Is it a runtime setting or build-time (the latter have a dedicated section)?
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/maintenance/NoSqlMaintenanceCommand.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * 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.admintool.maintenance;
+
+import picocli.CommandLine;
+
+@CommandLine.Command(
+ name = "maintenance",
+ subcommands = {
+ NoSqlMaintenanceLogCommand.class,
+ NoSqlMaintenanceRunCommand.class,
+ },
+ mixinStandardHelpOptions = true,
+ description = "Polaris persistence maintenance.")
Review Comment:
Should we flag it as "beta" as discussed in the last community sync call?.. or perhaps "preview" since the actually persistence storage that can be maintained via this command has not been hooked up yet (IIRC).
##########
runtime/admin/src/main/resources/application.properties:
##########
@@ -50,10 +67,13 @@ quarkus.datasource.devservices.enabled=false
quarkus.arc.ignored-split-packages=\
org.apache.polaris.admintool.config,\
+ org.apache.polaris.admintool.maintenance,\
Review Comment:
Could we refactor this code to avoid split packages upfront?
##########
runtime/admin/src/main/resources/application.properties:
##########
@@ -40,7 +40,24 @@ quarkus.rds.sync-client.type=apache
# ---- Runtime Configuration ----
# Below are default values for properties that can be changed in runtime.
+# Available types:
+# - in-memory - InMemoryPolarisMetaStoreManagerFactory
+# - in-memory-atomic - InMemoryAtomicOperationMetaStoreManagerFactory
+# - relational-jdbc - JdbcMetaStoreManagerFactory
+# - nosql - NoSQL persistence backend, define the backend type via 'polaris.persistence.nosql.backend'
polaris.persistence.type=relational-jdbc
+# Database backend for 'nosql' persistence-type
+# Available backends:
+# - InMemory - for testing purposes
+# - MongoDb - configure the via the Quarkus extension
+polaris.persistence.nosql.backend=InMemory
+
+## MongoDB version store specific configuration
Review Comment:
`MongoDB version store specific configuration defaults`? I suppose the user can change any of these settings.
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/maintenance/NoSqlMaintenanceRunCommand.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.admintool.maintenance;
+
+import jakarta.inject.Inject;
+import org.apache.polaris.persistence.nosql.maintenance.api.MaintenanceService;
+import picocli.CommandLine;
+
+@CommandLine.Command(
+ name = "run",
+ mixinStandardHelpOptions = true,
+ description = {"Run Polaris persistence maintenance."})
Review Comment:
maybe `Run Polaris NoSQL persistence maintenance` for the sake of clarity?
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3395:
URL: https://github.com/apache/polaris/pull/3395#discussion_r2677031654
##########
runtime/admin/src/main/java/org/apache/polaris/admintool/maintenance/NoSqlMaintenanceCommand.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * 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.admintool.maintenance;
+
+import picocli.CommandLine;
+
+@CommandLine.Command(
+ name = "maintenance",
+ subcommands = {
+ NoSqlMaintenanceLogCommand.class,
+ NoSqlMaintenanceRunCommand.class,
+ },
+ mixinStandardHelpOptions = true,
+ description = "Polaris persistence maintenance.")
Review Comment:
Should we flag it as "beta" as discussed in the last community sync call?.. or perhaps "preview" since the actual persistent storage that can be maintained via this command has not been hooked up yet (IIRC).
--
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: Add maintenance service to Polaris admin tool [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on PR #3395:
URL: https://github.com/apache/polaris/pull/3395#issuecomment-3737612674
The admin-tool is meant for all persistence implementations, because it is a tool to perform _administrative_ tasks. We have discussed before that other persistence specific operations like schema setup for JDBC would go into the admin-tool.
Adding another tool is rather going to complicate user experience.
--
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