Quarkus-based Polaris service runtime #469

adutra · 2024-11-22T18:36:46Z

Fixes #392.

Read the README_quarkus.md file for more details on what changes, how, and what remains to be done.

Having documentation about the configuration that is both "up-to-date" and "in-sync with the source code" with the actual code is good practice. The functionality added in this change allows generating markdown docs from smallrye-config based configuration types, starting at and grouped by types that are annotated with `@ConfigMapping`. The javadoc of the config interfaces and properties is emitted as markdown. On top there is functionality to generate reference docs for "static" types (currently unused in Polaris). The code is nearly a 1:1 port of the same functionality in Nessie. This change will become useful with/after apache#469. Gradle projects added in this change are: * `polaris-config-doc-annotations` adds an annotation used when generating docs to influence the generation of the configuration reference docs. * `polaris-config-doc-generator` is the reference doc generator implementation, as a "standalone" tool that uses javadoc _infrastructure_. See `ReferenceConfigDocsGenerator` why it needs to be a separate tool than "just" a doclet (class loader isolation issues). * `polaris-site-reference-docs` actually generates the markdown files from referenced Gradle projects. Currently there are no projects referenced, so generation yields nothing. The generated docs are not yet integrated into the website, which will happen in a follow-up PR.

dimas-b · 2025-01-13T19:13:49Z

integration-tests/src/main/java/org/apache/polaris/service/it/env/CatalogApi.java

-      queryParams.put("parent", RESTUtil.encodeNamespace(parent));
+      // TODO change this for Iceberg 1.7.2:
+      //   queryParams.put("parent", RESTUtil.encodeNamespace(parent));
+      queryParams.put("parent", Joiner.on('\u001f').join(parent.levels()));


Noted! Change LGTM!

dropwizard/service/README-quarkus.md

dimas-b · 2025-01-13T19:57:00Z

...rc/test/java/org/apache/polaris/service/dropwizard/it/QuarkusRestCatalogIntegrationTest.java

 import org.apache.polaris.service.it.test.PolarisRestCatalogIntegrationTest;

-public class DropwizardRestCatalogIntegrationTest extends PolarisRestCatalogIntegrationTest {}
+@QuarkusTest


nit: Do you think it should be @QuarkusIntegrationTest in the spirit of backbox tests?

Yes, but later: we don't have an intTest configuration in polaris yet :-)

Just add this to your build.gradle.kts (after #714)

testing { suites { val intTest by registering(JvmTestSuite::class) } } tasks.named("check") { dependsOn(testing.suites.named("intTest")) }

Nice, let's do it in a follow-up PR after the renamings etc.

When editing `.gitignore`, IntelliJ warns about a couple directories being ignored but not excluded from being indexed and searched. This change ignores the mentioned directories, except the one for DW, which is going away with apache#469.

Having documentation about the configuration that is both "up-to-date" and "in-sync with the source code" with the actual code is good practice. The functionality added in this change allows generating markdown docs from smallrye-config based configuration types, starting at and grouped by types that are annotated with `@ConfigMapping`. The javadoc of the config interfaces and properties is emitted as markdown. On top there is functionality to generate reference docs for "static" types (currently unused in Polaris). The code is nearly a 1:1 port of the same functionality in Nessie. This change will become useful with/after apache#469. Gradle projects added in this change are: * `polaris-config-doc-annotations` adds an annotation used when generating docs to influence the generation of the configuration reference docs. * `polaris-config-doc-generator` is the reference doc generator implementation, as a "standalone" tool that uses javadoc _infrastructure_. See `ReferenceConfigDocsGenerator` why it needs to be a separate tool than "just" a doclet (class loader isolation issues). * `polaris-site-reference-docs` actually generates the markdown files from referenced Gradle projects. Currently there are no projects referenced, so generation yields nothing. The generated docs are not yet integrated into the website, which will happen in a follow-up PR.

Having documentation about the configuration that is both "up-to-date" and "in-sync with the source code" with the actual code is good practice. The functionality added in this change allows generating markdown docs from smallrye-config based configuration types, starting at and grouped by types that are annotated with `@ConfigMapping`. The javadoc of the config interfaces and properties is emitted as markdown. On top there is functionality to generate reference docs for "static" types (currently unused in Polaris). The code is nearly a 1:1 port of the same functionality in Nessie. This change will become useful with/after #469. Gradle projects added in this change are: * `polaris-config-doc-annotations` adds an annotation used when generating docs to influence the generation of the configuration reference docs. * `polaris-config-doc-generator` is the reference doc generator implementation, as a "standalone" tool that uses javadoc _infrastructure_. See `ReferenceConfigDocsGenerator` why it needs to be a separate tool than "just" a doclet (class loader isolation issues). * `polaris-site-reference-docs` actually generates the markdown files from referenced Gradle projects. Currently there are no projects referenced, so generation yields nothing. The generated docs are not yet integrated into the website, which will happen in a follow-up PR.

flyrain · 2025-01-14T01:01:32Z

polaris-core/src/main/java/org/apache/polaris/core/context/CallContext.java

+    String realmId = base.getRealmContext().getRealmIdentifier();
+    RealmContext realmContext = () -> realmId;
+    PolarisCallContext polarisCallContext = PolarisCallContext.copyOf(base.getPolarisCallContext());


Nit: there are new comments in the caller, could we also add comments here to explain why we need cloned objects realmContext and polarisCallContext.

@flyrain this will soon disappear with #589 so I don't think there is need to explain.

But just for posterity, the reason is realted to TaskExecutor: these beans are request-scoped, and they would become unusable when the original request that created them completes, but the task hasn't finished executing yet.

flyrain · 2025-01-14T01:28:17Z

service/common/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java

@@ -107,12 +114,7 @@ private PolarisAdminService newAdminService(
    PolarisMetaStoreManager metaStoreManager =
        metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContext);
    return new PolarisAdminService(


Question: are we going to use DI for the class PolarisAdminService later as well?

Probably. There is a huge overlap between PolarisServiceImpl (which is a request-scoped CDI bean) and PolarisAdminService, which is just a regular Pojo.

flyrain · 2025-01-14T01:31:29Z

service/common/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java

-    PolarisCallContext polarisCallContext = CallContext.getCurrentContext().getPolarisCallContext();
+    PolarisCallContext polarisCallContext = callContext.getPolarisCallContext();


I suggest postponing this change until we've fixed line 101.

Let's keep this PR as it is - there is already #463 to get rid of the problematic TLs

flyrain · 2025-01-14T01:31:58Z

service/common/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java

-        metaStoreManager,
-        securityContext,
-        polarisAuthorizer);
+        callContext, entityManager, metaStoreManager, securityContext, polarisAuthorizer);


I suggest postponing this change until we've fixed the line 101.

flyrain · 2025-01-14T01:44:20Z

service/common/build.gradle.kts

+  implementation(libs.jakarta.annotation.api)
+  implementation(libs.jakarta.enterprise.cdi.api)
+  implementation(libs.jakarta.inject.api)
+  implementation(libs.jakarta.servlet.api)
+  implementation(libs.jakarta.validation.api)
+  implementation(libs.jakarta.ws.rs.api)


Considering this is a service-common module, do we need these libs in runtime classpaths?

I will take a note to check if these would work with compileOnly.

flyrain · 2025-01-14T02:24:35Z

dropwizard/service/README-quarkus.md

+* system properties
+* environment variables
+* `.env` file in the current working directory
+* `$PWD/config/application.properties` file
+* the `application.properties` packaged in the `polaris-quarkus-service` application


Can we improve the documentation to provide clearer and more detailed descriptions like the following:

Quarkus aggregates configuration properties from multiple sources, applying them in a specific order of precedence. When a property is defined in multiple sources, the value from the source with the higher priority overrides those from lower-priority sources. This behavior replaces the old configuration file format, such as polaris-server.yml.

The sources are listed below, from highest to lowest priority:

System properties: Properties set via the Java command line using -Dproperty.name=value.

Environment variables in .env file of the current working directory.

Settings in $PWD/config/application.properties file

The application configuration file packaged as part of the lib polaris-quarkus-service.

Default values: Hardcoded defaults within the application.

@flyrain this README is temporary, there is #700 to completely redesign the documentation.

There is already a paragraph about Quarkus configuration in #700, but I will make sure to include your suggestion above.

Incorporated the above changes in #700. FYI.

flyrain · 2025-01-14T02:51:44Z

dropwizard/service/README-quarkus.md

+
+* Bean injection (CDI) is made using `@ApplicationScoped` annotation on class and injected in other classes using `@Inject` annotation (https://quarkus.io/guides/cdi-reference). 
+* Codehale metrics registry and opentelemetry boilerplate (prometheus exporter included) are not needed anymore: Quarkus provides it "out of the box" (https://quarkus.io/guides/opentelemetry)
+* `PolarisHealthCheck` is not needed anymore: Quarkus provides it "out of the box" (https://quarkus.io/guides/smallrye-health)


I believe this potentially breaks existing deployment as the endpoint url has changed, like this example shows:

"http://localhost:8182/healthcheck" --> "http://localhost:8182/q/health"

I'd recommend to keep the existing health check in this PR. Deprecated it later if needed. cc @dennishuo @collado-mike @eric-maynard

@flyrain if you really have an existing deployment and need the old endpoint, then you can add it to your build (as a system integrator you likely have your own build).

I don't think this is a reasonable response to proposed breaking changes. This is a bit like saying "if you have your own fork, you can patch it". It is a reasonable use case that someone would check out the Polaris repo, package it up in a Docker build and deploy it to their K8s application and use the provided health checker endpoint to monitor it. We wouldn't remove one of the management APIs out from under users and tell them to just fix it in their own build. I don't think the health checker or the metrics APIs ought to be treated any differently

We did not remove the health check endpoint nor the metrics endpoint.

@collado-mike : Does #782 help?

When editing `.gitignore`, IntelliJ warns about a couple directories being ignored but not excluded from being indexed and searched. This change ignores the mentioned directories, except the one for DW, which is going away with #469.

snazy · 2025-01-14T10:28:49Z

gradle/libs.versions.toml

 picocli = "4.7.6"
 slf4j = "2.0.16"
 swagger = "1.6.14"

 [bundles]
+junit-testing = ["assertj-core", "mockito-core", "mockito-junit-jupiter", "junit-jupiter-api", "junit-jupiter-params", "junit-platform-reporting"]


I'll do a follow-up to adopt it to what #714 introduced

adutra force-pushed the polaris-quarkus-replacement branch from f714c40 to dcf8ccd Compare November 23, 2024 16:25

adutra mentioned this pull request Nov 25, 2024

Enhanced Apache Polaris runtime #392

Closed

adutra force-pushed the polaris-quarkus-replacement branch 4 times, most recently from 2f18194 to adfbbc2 Compare November 25, 2024 16:38

snazy mentioned this pull request Nov 25, 2024

Dynamic Config Resolver #459

Open

8 tasks

adutra force-pushed the polaris-quarkus-replacement branch 7 times, most recently from 4ac2974 to 5cc6fec Compare November 28, 2024 10:42

adutra changed the title ~~[WIP] Quarkus-based Polaris service runtime~~ Quarkus-based Polaris service runtime Nov 28, 2024

adutra force-pushed the polaris-quarkus-replacement branch 13 times, most recently from fe8b9dc to 9fd4ce3 Compare November 29, 2024 16:16

snazy mentioned this pull request Dec 2, 2024

Working impl of HK2 dependency injection #493

Merged

11 tasks

snazy mentioned this pull request Dec 16, 2024

Fix LICENSE/LICENSE.bin for release #495

Merged

adutra force-pushed the polaris-quarkus-replacement branch 3 times, most recently from a4e8638 to ea66fd4 Compare January 13, 2025 19:08

dimas-b reviewed Jan 13, 2025

View reviewed changes

eric-maynard reviewed Jan 13, 2025

View reviewed changes

dropwizard/service/README-quarkus.md Show resolved Hide resolved

dimas-b approved these changes Jan 13, 2025

View reviewed changes

snazy mentioned this pull request Jan 13, 2025

IntelliJ: exclude some directories from being indexed #720

Merged

adutra force-pushed the polaris-quarkus-replacement branch from ea66fd4 to c4e6ff0 Compare January 13, 2025 20:33

snazy mentioned this pull request Jan 13, 2025

Make tests work with Java 23 #723

Merged

flyrain reviewed Jan 14, 2025

View reviewed changes

Quarkus-based Polaris service runtime

e15f55a

adutra force-pushed the polaris-quarkus-replacement branch from c4e6ff0 to e15f55a Compare January 14, 2025 10:23

snazy reviewed Jan 14, 2025

View reviewed changes

snazy approved these changes Jan 14, 2025

View reviewed changes

adutra merged commit 4c7ae00 into apache:main Jan 14, 2025
5 checks passed

adutra deleted the polaris-quarkus-replacement branch January 14, 2025 10:38

This was referenced Jan 15, 2025

Dropwizard/HK2 service implementation doesn't bootstrap #565

Closed

Polaris not starting up / TaskHandlerConfiguration not JSON-serializable #561

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Quarkus-based Polaris service runtime #469

Quarkus-based Polaris service runtime #469

adutra commented Nov 22, 2024 •

edited

Loading

dimas-b Jan 13, 2025

dimas-b Jan 13, 2025

adutra Jan 13, 2025

snazy Jan 13, 2025

adutra Jan 13, 2025

dimas-b Jan 13, 2025

flyrain Jan 14, 2025

adutra Jan 14, 2025

flyrain Jan 14, 2025

adutra Jan 14, 2025

flyrain Jan 14, 2025

snazy Jan 14, 2025

flyrain Jan 14, 2025

flyrain Jan 14, 2025

adutra Jan 14, 2025

flyrain Jan 14, 2025

adutra Jan 14, 2025

adutra Jan 14, 2025

flyrain Jan 14, 2025

snazy Jan 14, 2025

collado-mike Jan 14, 2025

adutra Jan 15, 2025

dimas-b Jan 15, 2025

snazy Jan 14, 2025

		PolarisCallContext polarisCallContext = CallContext.getCurrentContext().getPolarisCallContext();
		PolarisCallContext polarisCallContext = callContext.getPolarisCallContext();

Quarkus-based Polaris service runtime #469

Quarkus-based Polaris service runtime #469

Conversation

adutra commented Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adutra commented Nov 22, 2024 •

edited

Loading