NoSQL: Add maintenance service to Polaris admin tool by snazy · Pull Request #3395 · apache/polaris

snazy · 2026-01-09T10:30:15Z

This change wires the maintenance service up to the Polaris admin tool.

dimas-b

LGTM 👍 just a few minor comments 🙂

dimas-b · 2026-01-09T17:32:15Z

...me/admin/src/main/java/org/apache/polaris/admintool/maintenance/NoSqlMaintenanceCommand.java

+      NoSqlMaintenanceRunCommand.class,
+    },
+    mixinStandardHelpOptions = true,
+    description = "Polaris persistence maintenance.")


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).

I think we also probably shouldn't call it persistence maintenance unless we plan on having maintenance for other backends?

dimas-b · 2026-01-09T17:34:35Z

...src/main/java/org/apache/polaris/admintool/nosql/maintenance/NoSqlMaintenanceRunCommand.java

+@CommandLine.Command(
+    name = "run",
+    mixinStandardHelpOptions = true,
+    description = {"Run Polaris persistence maintenance."})


maybe Run Polaris NoSQL persistence maintenance for the sake of clarity?

@snazy : WDYT?

... and similar description changes in the other new commands?

dimas-b · 2026-01-09T17:36:09Z

runtime/admin/src/main/resources/application.properties

+# - MongoDb - configure the via the Quarkus extension
+polaris.persistence.nosql.backend=InMemory
+
+## MongoDB version store specific configuration


MongoDB version store specific configuration defaults? I suppose the user can change any of these settings.

Same comments request here as on the service pr, These properties should probably be commented out by default since they aren't used

dimas-b · 2026-01-09T17:36:57Z

runtime/admin/src/main/resources/application.properties

+quarkus.mongodb.database=polaris
+quarkus.mongodb.metrics.enabled=true
+#quarkus.mongodb.connection-string=mongodb://localhost:27017
+quarkus.mongodb.devservices.enabled=false


Is it a runtime setting or build-time (the latter have a dedicated section)?

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.

dimas-b · 2026-01-09T17:37:35Z

runtime/admin/src/main/resources/application.properties


 quarkus.arc.ignored-split-packages=\
  org.apache.polaris.admintool.config,\
+  org.apache.polaris.admintool.maintenance,\


Could we refactor this code to avoid split packages upfront?

Note: this package no longer exists.

Package org.apache.polaris.admintool.nosql.maintenance only exists in one module, that is in runtime/admin... Is this config required anymore? 🤔

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]

I've added a commit to remove this prop.

I'll deal with the other warnings on main later.

flyrain

I do not think the NoSQL maintenance tool belongs in the runtime admin module. NoSQL is just one persistence impl., and this maintenance logic is only applicable to that specific backend. Other persistence implementations do not need it, yet this change pulls NoSQL specific behavior into a shared admin layer. This creates unnecessary coupling, complicates the module boundaries, and makes the admin tooling harder to reason about and extend going forward.

cc @dennishuo @collado-mike @singhpk234 @jbonofre @HonahX @RussellSpitzer

snazy · 2026-01-12T09:28:14Z

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.

dimas-b · 2026-01-12T15:18:42Z

@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.

I believe this is the minimum the project community can do for OSS users.

singhpk234

NoSQL maintenance service introduces constructs which are not agreed upon in Apache Polaris, neither have ever been discussed and agreed upon the community. for example retention expression, dependency on an unmaintained project nessie library here mentioned here #3268 (review), other than question to where this should be in Polaris tools or Polaris repo has not been agreed upon, despite being called out.

It has been requested to start dev thread discussion to establish concencus for the same, despite that underlying PR was merged
#3268 (comment)

To avoid any confusion and explicitly mark my objection i am requesting changes, kindly please start thread establish concencus upon the semantics that Polaris should support .

dimas-b · 2026-01-14T22:28:18Z

Related dev discussion: https://lists.apache.org/thread/znw87l7r1gh3l09pc0hxhj5gyl8vp28d

RussellSpitzer · 2026-01-16T19:33:54Z

...me/admin/src/main/java/org/apache/polaris/admintool/maintenance/NoSqlMaintenanceCommand.java

+    out.println("use the 'help maintenance' command.");
+    out.println();
+
+    checkInMemory();


Shouldn't we be failing for everything that isn't NoSql not just InMemory?

RussellSpitzer · 2026-01-16T19:51:20Z

...me/admin/src/main/java/org/apache/polaris/admintool/maintenance/NoSqlMaintenanceCommand.java

+    checkInMemory();
+
+    out.println();
+    out.println("Information: selected NoSql persistence backend: " + backend.type());


In the current formulation this isn't correct since you can call this with any backend right?

RussellSpitzer · 2026-01-16T19:54:25Z

...ime/admin/src/main/java/org/apache/polaris/admintool/maintenance/BaseMaintenanceCommand.java

+  @Inject protected MaintenanceConfig maintenanceConfig;
+
+  protected void checkInMemory() {
+    if ("InMemory".equals(backend.type())) {


Do we need a check here for the persistence not being NoSQL?

RussellSpitzer · 2026-01-16T20:16:30Z

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.

RussellSpitzer · 2026-01-22T19:44:37Z

runtime/admin/src/main/java/org/apache/polaris/admintool/nosql/NoSqlCommand.java

+import picocli.CommandLine;
+
+@CommandLine.Command(
+    name = "nosql",


RussellSpitzer

LGTM +1, I notice there are still some comments from @dimas-b though that are still not addressed?

I can agree to disagree here, I don't need to agree here either (dismissing my request changes), as we definitely want to, imho this tool doesn't belong in admin module, specially strictly speaking from the MetaStorageManager interface POV bootstrap and purge are well defined in this manage interface. and admin tool documentation its for metastore management.

dimas-b · 2026-01-31T00:22:59Z

runtime/admin/src/main/java/org/apache/polaris/admintool/nosql/NoSqlCommand.java

+      NoSqlMaintenanceRunCommand.class,
+    },
+    mixinStandardHelpOptions = true,
+    description = "Polaris NoSQL persistence.")


nit: suggestion: Sub-commands specific to NoSQL persistence.

dimas-b · 2026-01-31T00:25:04Z

...rc/main/java/org/apache/polaris/admintool/nosql/maintenance/NoSqlMaintenanceInfoCommand.java

+import picocli.CommandLine;
+
+@CommandLine.Command(
+    name = "maintenance-info",


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.

dimas-b · 2026-01-31T00:27:31Z

runtime/admin/src/main/resources/application.properties

+quarkus.mongodb.database=polaris
+quarkus.mongodb.metrics.enabled=true
+#quarkus.mongodb.connection-string=mongodb://localhost:27017
+quarkus.mongodb.devservices.enabled=false


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.

dimas-b · 2026-01-31T00:40:04Z

runtime/admin/src/main/resources/application.properties

+quarkus.index-dependency.agrona.group-id=org.agrona
+quarkus.index-dependency.agrona.artifact-id=agrona


I tried building locally without these settings (with the workaround to get build warnings), but agrona was not flagged as needing an index... Is this setting required for the admin tool?

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?

dimas-b · 2026-01-31T00:54:18Z

Summarizing the dev discussion.

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 enables. 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.

dimas-b · 2026-01-31T00:55:44Z

@flyrain : Would you please reconsider your -1 on this PR given the above discussion?

dimas-b · 2026-01-31T00:57:15Z

@RussellSpitzer : just FYI: I re-requested your review my accident 😅 but fresh comments are welcome, of course 🙂

dimas-b · 2026-02-02T16:07:43Z

@snazy : could you rebase 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 🤔

snazy · 2026-02-02T16:26:52Z

Rebased + force-pushed

runtime/admin/src/main/resources/application.properties

dimas-b

I believe this PR is good to merge. We can deal with minor remaining concerns on main after merging.

dimas-b · 2026-02-04T18:18:49Z

@flyrain : Could you review again, please? There were code updates and fresh discussion points after your previous -1 review.

flyrain · 2026-02-05T02:24:49Z

Sorry for the late reply, I have posted the concern in the dev ML https://lists.apache.org/thread/93nvjwwbvv77fz16rsdcwsm1m8nc5trt

This change wires the maintenance service up to the Polaris admin tool.

snazy requested a review from dimas-b January 9, 2026 10:30

github-project-automation bot added this to Basic Kanban Board Jan 9, 2026

github-project-automation bot moved this to PRs In Progress in Basic Kanban Board Jan 9, 2026

dimas-b reviewed Jan 9, 2026

View reviewed changes

flyrain requested changes Jan 10, 2026

View reviewed changes

flyrain requested a review from dennishuo January 10, 2026 20:47

dimas-b mentioned this pull request Jan 12, 2026

NoSQL: Metastore maintenance #3268

Merged

singhpk234 previously requested changes Jan 12, 2026

View reviewed changes

RussellSpitzer reviewed Jan 16, 2026

View reviewed changes

RussellSpitzer reviewed Jan 22, 2026

View reviewed changes

RussellSpitzer previously approved these changes Jan 22, 2026

View reviewed changes

dimas-b reviewed Jan 31, 2026

View reviewed changes

dimas-b requested review from RussellSpitzer and flyrain January 31, 2026 00:55

dimas-b added this to the 1.4.0 milestone Jan 31, 2026

snazy force-pushed the nosql-maintenance-admin branch from 292cec4 to 3472306 Compare February 2, 2026 16:23

snazy dismissed RussellSpitzer’s stale review via a1a65a6 February 2, 2026 16:25

snazy force-pushed the nosql-maintenance-admin branch from 3472306 to a1a65a6 Compare February 2, 2026 16:25

dimas-b reviewed Feb 3, 2026

View reviewed changes

runtime/admin/src/main/resources/application.properties Outdated Show resolved Hide resolved

dimas-b approved these changes Feb 3, 2026

View reviewed changes

snazy force-pushed the nosql-maintenance-admin branch 2 times, most recently from 0914824 to f5e0d40 Compare February 4, 2026 09:14

This was referenced Feb 5, 2026

feat(persistence): Add JDBC persistence layer for Iceberg metrics reporting (#3337) #3385

Open

Add MongoDB persistence backend #650

Open

snazy and others added 5 commits February 10, 2026 10:30

NoSQL: Add maintenance service to Polaris admin tool

988ed97

This change wires the maintenance service up to the Polaris admin tool.

update/relocate nosql commands

cd00ec5

config updates

4017f53

rm

cfba813

review: remove unnecessary property

a87bfdc

snazy force-pushed the nosql-maintenance-admin branch from f5e0d40 to a87bfdc Compare February 10, 2026 09:31

		quarkus.index-dependency.agrona.group-id=org.agrona
		quarkus.index-dependency.agrona.artifact-id=agrona

Conversation

snazy commented Jan 9, 2026

Uh oh!

dimas-b left a comment

Choose a reason for hiding this comment

Uh oh!

dimas-b Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flyrain left a comment

Choose a reason for hiding this comment

Uh oh!

snazy commented Jan 12, 2026

Uh oh!

dimas-b commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

singhpk234 left a comment

Choose a reason for hiding this comment

Uh oh!

dimas-b commented Jan 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RussellSpitzer commented Jan 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RussellSpitzer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimas-b Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dimas-b commented Jan 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dimas-b commented Jan 31, 2026

dimas-b Jan 9, 2026 •

edited

Loading

dimas-b commented Jan 12, 2026 •

edited

Loading

dimas-b Feb 3, 2026 •

edited

Loading

dimas-b commented Jan 31, 2026 •

edited

Loading

dimas-b commented Feb 2, 2026 •

edited

Loading