Fix Production readiness for Persistence

[singhpk234]

About the change

Eclipse-link production readiness check should only run when the configured persistence type is eclipse-link but it turns out it doesn't and it creates a noise and confusion like #1684

Also add checks for JDBC production readiness

Before the change

➜  ~ docker logs jdbc-polaris-1 | grep "eclip"
INFO exec -a "java" java -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:5005 -XX:MaxRAMPercentage=80.0 -XX:+UseParallelGC -XX:MinHeapFreeRatio=10 -XX:MaxHeapFreeRatio=20 -XX:GCTimeRatio=4 -XX:AdaptiveSizePolicyWeight=90 -XX:+ExitOnOutOfMemoryError -cp "." -jar /deployments/quarkus-run.jar
INFO running in /deployments
2025-05-27 17:09:25,492 WARN  [org.apa.pol.ser.qua.con.ProductionReadinessChecks] [,] [,,,] (main) - :warning: The current persistence unit (jdbc:h2) is intended for tests only. Offending configuration option: 'polaris.persistence.eclipselink.configuration-file'.

After the change

➜  ~ docker logs jdbc-polaris-1 | grep "eclip"
INFO exec -a "java" java -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:5005 -XX:MaxRAMPercentage=80.0 -XX:+UseParallelGC -XX:MinHeapFreeRatio=10 -XX:MaxHeapFreeRatio=20 -XX:GCTimeRatio=4 -XX:AdaptiveSizePolicyWeight=90 -XX:+ExitOnOutOfMemoryError -cp "." -jar /deployments/quarkus-run.jar
INFO running in /deployments

[flyrain]

LGTM with a minor comment.

[dimas-b]

Why do we have to .select() here? The usual producer should normally yield a value if we just declared MetaStoreManagerFactory for injection, I think.

[singhpk234]

This is because both @Inject as a member of this class as well as using @Any Instance and then doing .get() letter was giving ambigous bean, I wonder if these checks run before the bean is actually created from QuarkusProducers, this is the only approach worked for me.

[dimas-b]

Hmmm, this WFM :)

  @Produces
  public ProductionReadinessCheck checkJdbcUrl(EclipseLinkConfiguration eclipseLinkConfiguration,
  MetaStoreManagerFactory f) {
    System.out.println("AAA: " + f.getClass());
[...]

Output: AAA: class org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory_ClientProxy

[singhpk234]

🤦‍♂️ I didn't try just passing it in the parameter without an Annotation. i though like anyother producer it would need one, I can confirm your suggestion works, though any explanation why even with @Produces annotation we don;t need any annotation ?

[dimas-b]

Quarkus CDI is able to figure out bean dependencies. When a producer method is needed for a bean, its inputs are automatically injected, I suppose.

[dimas-b]

This might be more reliable, plus it's in sync with actual persistence usage:

polaris/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java

Line 119 in 2a69415

databaseType = DatabaseType.fromDisplayName(productName);

The EclipseLink side is good to merge, IMHO, so if you prefer we could move this to a separate PR.

[dimas-b]

I would not mind adding this: JdbcMetaStoreManagerFactory.getDatabaseType()

[singhpk234]

sounds good, tbh i think the h2 check is fine, though one thing i realized doing this will also check the connection is properly made to the DB and if its not then we simply fail early that your DB is misconfigured correct it !

let me make the change should be small IMHO

[dimas-b]

Good point about connections. Although, I would not fail the startup in that case, maybe just produce a warning check?

[dimas-b]

In general, connection checks should be part of the "readiness" check.

[singhpk234]

Agree, though i prefer failing as without persistence we can't use the application at all, let me know if you feel strongly otherwise.

[dimas-b]

LGTM 👍 Thanks for bearing with me, @singhpk234 :)

[singhpk234]

Thank you so much for review everyone !