Make MPCONFIG usable in *all* DataSourceDefinition properties / FISH-862

[poikilotherm]

Using a data source definition in web.xml or @DataSourceDefinition does not allow to use ${MPCONFIG=...} (or ${ENV}) for Payara specific options ("fish.payara.xxx") or integer values used i. e. for maxIdleTime.

This should be changed to allow for more flexible configuration and fine-tuning in deployments.

Expected Outcome

Successfull deployment with JDBC pools configured via MicroProfile Config API for any kind of setting. Usually non-portable, but most options aren't portable anyway.

Provide an option to set the integer based pool settings (minPoolSize, maxPoolSize, ...) from a Payara specific properties setting using a String and thus enable variable substitution. (e. g. fish.payara.max-pool-size, ...)

Current Outcome

When using a data source like

@DataSourceDefinition(
    name = "java:app/jdbc/dataverse",
    className = "org.postgresql.ds.PGConnectionPoolDataSource",
    user = "${MPCONFIG=dataverse.db.user}",
    password = "${MPCONFIG=dataverse.db.password}",
    url = "jdbc:postgresql://${MPCONFIG=dataverse.db.host}:${MPCONFIG=dataverse.db.port}/${MPCONFIG=dataverse.db.name}",
    minPoolSize = 10,
    maxPoolSize = 100,
    maxIdleTime = 300,
    properties = {
        "fish.payara.pool-resize-quantity=${MPCONFIG=dataverse.db.pool-resize-quantity}",
        "fish.payara.is-connection-validation-required=${MPCONFIG=dataverse.db.is-connection-validation-required}",
        "fish.payara.connection-validation-method=${MPCONFIG=dataverse.db.connection-validation-method}",
        "fish.payara.validation-table-name=${MPCONFIG=dataverse.db.validation-table-name}"
    }
)

this will fail to deploy with quite generic errors:

[#|2020-12-02T12:43:58.635+0000|WARNING|Payara 5.2020.6|javax.enterprise.resource.resourceadapter.org.glassfish.jdbc.deployer|_ThreadID=1;_ThreadName=main;_TimeMillis=1606913038635;_LevelValue=900;|
  error.creating.jdbc.pool|#]

Context (Optional)

The idea is to enable Dataverse admins to configure certain things like monitoring for slow queries etc by configuring simple settings via MicroProfile Config API.

Digging through

• DataSourceDefinitionHandler, DataSourceDefinitionDescriptor and DataSourceDefinitionDeployer
• leading me to DataSourceSpec, JdbcConnectionPoolDeployer and ConnectorConfigProperty

I come to the conclusion, that MPCONFIG (or ENV or ALIAS) is only usable for options using TranslatedConfigView in JdbcConnectionPoolDeployer.getMCFConfigProperties(). (Please correct me if I'm missing sth. here.)

Environment

• Payara Version: 5.2020.6
• Edition: Full
• JDK Version: 8
• Operating System: Linux
• Database: PostgreSQL

[AlanRoth]

Hi, @poikilotherm,

Thank you for your enhancement request.

I have raised an internal issue 'FISH-862', however, I will stress that this feature request may take a long time before it is worked on by our engineering team, due to the sheer number of requests and our priority being customer requests.

I highly recommend that you attempt to implement this feature yourself in a community contribution, as it may be faster in that sense.

Thank you,
Alan

[poikilotherm]

Hi @AlanRoth
Thanks for coming back to me.
I'd be happy to create a contribution.

Yet I would love some guidance to avoid misconcepts and heading down the wrong road. This is also a TCK relevant part and I don't want people to fix bugs I created in this code largely dated back to 2011.

[jbee]

@poikilotherm Every help is welcome. I think you are on the right track with TranslatedConfigView being responsible for the replacement of MP config properties but the exact interaction and involved parties are quite delicate in this case. I think you should wait for @Pandrex247 giving you pointers when he is back next week.

[Pandrex247]

TranslatedConfigView is indeed the best bet for grabbing aliased properties.
I don't know the deployment codepath off the top of my head (yet), but JdbcConnectionPoolDeployer#getMCFConfigProperties does seem to be a good place to try and plug it in since it's already used for a few properties in that method.

[smillidge]

The only thing I would be concerned about in that area is there is also bean validation on the properties. It may be that it won't work with non-String properties but I can't quite remember.

[smillidge]

A very simple fix for the annotation only is likely in the DataSourceDefinition annotation handler here
https://github.com/payara/Payara/blob/master/appserver/jdbc/jdbc-runtime/src/main/java/org/glassfish/jdbcruntime/deployment/annotation/handlers/DataSourceDefinitionHandler.java#L457
push the value through TranslatedConfigView probably also needs doing in merge here https://github.com/payara/Payara/blob/master/appserver/jdbc/jdbc-runtime/src/main/java/org/glassfish/jdbcruntime/deployment/annotation/handlers/DataSourceDefinitionHandler.java#L360

Another good enhancement could be hijacking the properties section of the annotation and loading a specified properties file using

Properties props = Properties.load(this.getClass().getResourceAsStream(...));

as this would enable changing properties without recompiling code e.g. the definition could be;

@DataSourceDefinition(
    name = "java:app/jdbc/dataverse",
    className = "org.postgresql.ds.PGConnectionPoolDataSource",
    user = "${MPCONFIG=dataverse.db.user}",
    password = "${MPCONFIG=dataverse.db.password}",
    url = "jdbc:postgresql://${MPCONFIG=dataverse.db.host}:${MPCONFIG=dataverse.db.port}/${MPCONFIG=dataverse.db.name}",
    minPoolSize = 10,
    maxPoolSize = 100,
    maxIdleTime = 300,
    properties = {
        "properties-to-load.properties"
    }
)

[AlanRoth]

Hi @poikilotherm, starting today we are introducing some relevant changes to our Github Issues Community Policy, which you can read here. The most significant change is that we want to introduce open votes for all suggested enhancements and improvements to the Payara Platform, like the one described in this issue. To this effect we are going to transition this issue to Voting status and gauge the community interest on your proposal. We know that previously we accepted your requested and escalated it to the our Platform Development team, but after careful deliberation and analysis we want to improve our community engagement in a way that’s productive and relevant for our overall user community. We hope that you understand this decision.

[Lars5678]

I think the idea is very good that, in addition to the previous configuration options, i can also define integer values ​​or payara-specific properties via the MP Config.

We have different database configurations (e.g. PoolSize) in our different environments. Unfortunately it is impossible for us to define this via the elegant MP Config way.

So far we used asadmin commands in docker file for payara full depending on the enviroment.
With the switch to Eclipse Microprofile and Payara Micro, we hoped to be able to do without the Asadmin commands, which are different in the various environments. Unfortunately, this is currently not possible for us. We don't want to use different docker files and don't want to share configuration. We wan't the configuration at the same / one place... in this case in MP Config.

So we hope that in the future we will be able to define our environment-dependent database properties via the Miroprofile Config. Both for integer values ​​and Payara specific parameters.

[thehpi]

Hi @poikilotherm,

While working on a fix for #5059 I also tried the MPCONFIG configuration.
I found that when using payara micro and using a postboot command to set the config properties it works fine.
I also tested the validation settings from your example.

I also found that when you provide the settings by creating the microprofile-config.properties file in META-INF in your application it won't work.

The reason for this is that the 'application' config properties are only available after the application has been deployed.
The @DatasourceDefinition is processed when annotation processing is done, which happens during server startup, so the MPCONFIG properties are not available.

So if you used the META-INF/microprofile-config.properties approach then this is probable the reason.

[poikilotherm]

Hi @thehpi,

not so sure about the availability of values from META-INF/microprofile-config.properties during deployments. From my experience it should work, yet I would retest with the next Payara version that will print warnings about missing properties (which makes this much easier to test reliably).

Yet, this enhancement request is about adding support to configure properties that the Jakarta EE interface defines as integers (no way to override those by string substitution for most of 'em) and use MPCONFIG with the properties field entries.

Based on my code research, I'm very sure this is not possible yet, but it would be great to see your workaround.

[poikilotherm]

@thehpi I looked at #5142 and it does a lot more than just replacing the classname via var substitution... Thank you very much for your work on this!

@AlanRoth is this issue solved with the merge of that PR? (No it isn't...)

Looks like there is just one thing left from my enhancement request here: setting integer only properties like pool sizes etc.

@AlanRoth I'd be happy to add some code for this, using non-portable properties with prefix "fish.payara...." like the other special props. Sounds good?

[AlanRoth]

Hi @poikilotherm, sounds good. Submit a PR and we'll gladly have a look!

[thehpi]

Hi @poikilotherm,

Yes #5142 is accepted and merged into master. It fixes #5095 but also does ENV/MPCONFIG etc expansion in all other DatasourceDefinition settings but also in the properties. So if you have a property to replace the DatasourceDefintion setting for 'minPoolSize' etc, then you can use that in a new release.

So you could use something like:

@DataSourceDefinition(
        name = "java:global/myDS",
        className =     "${MPCONFIG=nl.thehpi.jdbc.db.class_name}",
        user =          "${MPCONFIG=nl.thehpi.jdbc.db.user}",
        password =      "${MPCONFIG=nl.thehpi.jdbc.db.password}",
        url =      "jdbc:${MPCONFIG=nl.thehpi.jdbc.db.type}:${MPCONFIG=nl.thehpi.jdbc.db.url}",
        properties = {
                "fish.payara.pool-resize-quantity=${MPCONFIG=nl.thehpi.jdbc.pool-resize-quantity}",
                "fish.payara.is-connection-validation-required=${MPCONFIG=nl.thehpi.jdbc.validation-required}",
                "fish.payara.connection-validation-method=${MPCONFIG=nl.thehpi.jdbc.validation-method}",
                "fish.payara.validation-table-name=${MPCONFIG=nl.thehpi.jdbc.validation-table-name}",
                "fish.payara.min-pool-size=${MPCONFIG=nl.thehpi.jdbc.db.minPoolSize}",
                "fish.payara.max-pool-size=${MPCONFIG=nl.thehpi.jdbc.db.maxPoolSize}",
                "fish.payara.idle-timeout-in-seconds=${MPCONFIG=nl.thehpi.jdbc.db.maxIdleTime}"
        }
)

And in payara you could use a post-boot-script like e.g.

set-config-property --propertyName=nl.thehpi.jdbc.db.type --propertyValue=postgresql --source=domain
set-config-property --propertyName=nl.thehpi.jdbc.db.class_name --propertyValue=org.postgresql.xa.PGXADataSource --source=domain
set-config-property --propertyName=nl.thehpi.jdbc.db.user --propertyValue=admin --source=domain
set-config-property --propertyName=nl.thehpi.jdbc.db.password --propertyValue=adminpwd --source=domain
set-config-property --propertyName=nl.thehpi.jdbc.db.url --propertyValue=//localhost:5432/database --source=domain

set-config-property --propertyName=nl.thehpi.jdbc.pool-resize-quantity --propertyValue=5 --source=domain

set-config-property --propertyName=nl.thehpi.jdbc.validation-required --propertyValue=true --source=domain
set-config-property --propertyName=nl.thehpi.jdbc.validation-method --propertyValue=table --source=domain
set-config-property --propertyName=nl.thehpi.jdbc.validation-table-name --propertyValue=testentity --source=domain

set-config-property --propertyName=nl.thehpi.jdbc.minPoolSize --propertyValue=1 --source=domain
set-config-property --propertyName=nl.thehpi.jdbc.maxPoolSize --propertyValue=5 --source=domain
set-config-property --propertyName=nl.thehpi.jdbc.maxIdleTime --propertyValue=10 --source=domain