[JDBC] [DO NOT REVIEW] Support multiple Quarkus datasources #1482
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| /* | ||
| * 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.core.persistence; | ||
|
|
||
| import javax.sql.DataSource; | ||
|
|
||
| public interface DatasourceSupplier { | ||
| DataSource fromRealmId(String realmId); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,3 +50,21 @@ quarkus.index-dependency.guava.artifact-id=guava | |
| quarkus.index-dependency.protobuf.group-id=com.google.protobuf | ||
| quarkus.index-dependency.protobuf.artifact-id=protobuf-java | ||
| quarkus.datasource.devservices.image-name=postgres:17-alpine | ||
|
|
||
| #quarkus.datasource.db-kind=pgsql | ||
| #quarkus.datasource.jdbc.url=polaris | ||
| #quarkus.datasource.username=polaris | ||
| #quarkus.datasource.password=polaris | ||
| quarkus.datasource.\"realm1_ds\".db-kind=pgsql | ||
|
There was a problem hiding this comment. I'm still not sure why we need the multitude of test-like datasource configs in the production sections of the admin tool properties 🤔 There was a problem hiding this comment. not sure whats happening but here is the reason behind : #1482 (comment) There was a problem hiding this comment. The problem here is that for a Datasource to be created by Quarkus, there has to be at least one build-time property defined for that datasource: And you cannot pass build-time properties through So Thus for tests you have to declare the datasources you intend to use here, along with their db-kind: quarkus.datasource.\"realm1_ds\".db-kind=pgsql
quarkus.datasource.\"realm2_ds\".db-kind=pgsql
quarkus.datasource.\"realm3_ds\".db-kind=pgsql
# etcOther runtime properties, like There was a problem hiding this comment. If I'm surprised this is required, because the default datasource does not have to be defined at build time, if I'm not mistaken. There was a problem hiding this comment.
See https://quarkus.io/guides/datasource#configure-multiple-datasources:
This is why I asked this question, I think there is some misunderstanding going on: #1482 (comment)
The default datasource is treated slightly different: if But that doesn't help here, since the intent is to use many named datasources. There was a problem hiding this comment. from my POV, it might be fine to support one Data Source (co-located realms) out-of-the-box and instruct users to use custom builds if they want to segregate realms by Data Source (experience similar to EclipseLink). There was a problem hiding this comment. Why not configure those DS using a quarkus-test-profile? There was a problem hiding this comment. @snazy we can't as we need atleast one build time prop please ref this comment from @adutra #1482 (comment) I tried overriding them by quarkus-test-profiles but the override is not able to create named DS There was a problem hiding this comment. @singhpk234 : This PR accumulated a lot of comments, some of them about non-trivial issues... Would you mind making a dev email with a summary and options for moving forward? I guess it would be clearer than continuing on GH. Once we have a consensus on how to proceed we can revamp this PR. WDYT? There was a problem hiding this comment. sounds fair ! its on my list, should be sending out an email soon, meanwhile marking this PR as draft to not draw more attention, |
||
| quarkus.datasource.\"realm1_ds\".jdbc.url=polaris | ||
| quarkus.datasource.\"realm1_ds\".username=polaris | ||
| quarkus.datasource.\"realm1_ds\".password=polaris | ||
| quarkus.datasource.\"realm2_ds\".db-kind=pgsql | ||
| quarkus.datasource.\"realm2_ds\".jdbc.url=polaris | ||
| quarkus.datasource.\"realm2_ds\".username=polaris | ||
| quarkus.datasource.\"realm2_ds\".password=polaris | ||
| quarkus.datasource.\"realm3_ds\".db-kind=pgsql | ||
| quarkus.datasource.\"realm3_ds\".jdbc.url=polaris | ||
| quarkus.datasource.\"realm3_ds\".username=polaris | ||
| quarkus.datasource.\"realm3_ds\".password=polaris | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| /* | ||
| * 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. | ||
| */ | ||
|
|
||
| -- Create two more databases for testing. The first database, polaris_realm1, is created | ||
| -- during container initialization. See PostgresTestResourceLifecycleManager. | ||
|
|
||
| -- Note: the database names must follow the pattern polaris_{realm}. That's the pattern | ||
| -- specified by the persistence.xml file used in tests. | ||
|
|
||
| CREATE DATABASE realm2; | ||
| GRANT ALL PRIVILEGES ON DATABASE realm2 TO polaris; | ||
|
|
||
| CREATE DATABASE realm3; | ||
| GRANT ALL PRIVILEGES ON DATABASE realm3 TO polaris; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| /* | ||
| * 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. | ||
| */ | ||
|
|
||
| plugins { | ||
| alias(libs.plugins.jandex) | ||
| id("java-test-fixtures") | ||
| } | ||
|
|
||
| configurations.all { | ||
| exclude(group = "org.antlr", module = "antlr4-runtime") | ||
| exclude(group = "org.scala-lang", module = "scala-library") | ||
| exclude(group = "org.scala-lang", module = "scala-reflect") | ||
| } | ||
|
|
||
| java { | ||
| sourceCompatibility = JavaVersion.VERSION_21 | ||
| targetCompatibility = JavaVersion.VERSION_21 | ||
| } | ||
|
|
||
| dependencies { | ||
| implementation(project(":polaris-core")) | ||
| implementation(project(":polaris-relational-jdbc")) | ||
|
|
||
| compileOnly(libs.smallrye.config.core) // @ConfigMap | ||
| implementation(enforcedPlatform(libs.quarkus.bom)) | ||
| implementation("io.quarkus:quarkus-arc") | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| /* | ||
| * 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.common; | ||
|
|
||
|
|
||
| import io.quarkus.arc.All; | ||
| import io.quarkus.arc.InstanceHandle; | ||
| import java.util.List; | ||
| import javax.sql.DataSource; | ||
|
|
||
| import jakarta.enterprise.context.ApplicationScoped; | ||
| import jakarta.inject.Inject; | ||
| import org.apache.polaris.core.persistence.DatasourceSupplier; | ||
|
|
||
| @ApplicationScoped | ||
| public class QuarkusDatasourceSupplier implements DatasourceSupplier { | ||
| private final List<InstanceHandle<DataSource>> dataSources; | ||
| private final RelationalJdbcConfiguration relationalJdbcConfiguration; | ||
|
|
||
| private static final String DEFAULT_DATA_SOURCE_NAME = "default"; | ||
|
|
||
| @Inject | ||
| public QuarkusDatasourceSupplier( | ||
| RelationalJdbcConfiguration relationalJdbcConfiguration, | ||
| @All List<InstanceHandle<DataSource>> dataSources) { | ||
| this.relationalJdbcConfiguration = relationalJdbcConfiguration; | ||
| this.dataSources = dataSources; | ||
| } | ||
|
|
||
| @Override | ||
| public DataSource fromRealmId(String realmId) { | ||
| // check if the mapping of realm to DS exists, otherwise fall back to default | ||
| String dataSourceName = relationalJdbcConfiguration.realms().getOrDefault( | ||
| realmId, | ||
| relationalJdbcConfiguration.defaultDatasource().orElse(null) | ||
| ); | ||
|
|
||
| // if neither mapping exists nor default DS exists, fail | ||
| if (dataSourceName == null) { | ||
| throw new IllegalStateException(String.format( | ||
| "No datasource configured with name: %s nor default datasource configured", realmId)); | ||
| } | ||
|
|
||
| // check if there is actually a datasource of that dataSourceName | ||
| return dataSources.stream() | ||
| .filter(ds -> { | ||
| String name = ds.getBean().getName(); | ||
| name = name == null ? DEFAULT_DATA_SOURCE_NAME : name; | ||
| return name.equals(dataSourceName); | ||
| }) | ||
| .map(InstanceHandle::get) | ||
| .findFirst() | ||
| .orElseThrow(() -> new IllegalStateException(String.format( | ||
| "No datasource configured with name: %s", dataSourceName))); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| /* | ||
| * 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.common; | ||
|
|
||
| import io.smallrye.config.ConfigMapping; | ||
| import io.smallrye.config.WithParentName; | ||
|
|
||
| import java.util.Map; | ||
| import java.util.Optional; | ||
|
|
||
| @ConfigMapping(prefix = "polaris.relational.jdbc.datasource") | ||
| public interface RelationalJdbcConfiguration { | ||
| /** realmId to configured Datasource name mapping. */ | ||
| @WithParentName | ||
| Map<String, String> realms(); | ||
|
|
||
| /** | ||
| * Default datasource name to be used for a realmId when there is no mapping of realmId to | ||
| * Datasource name present. | ||
| */ | ||
| Optional<String> defaultDatasource(); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So why not throw an exception in that case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its like we checked and table didn't exists, so this looks up should not fail but rather say that he nothing is bootstrappped and hence trigger actual bootstrap or purge or something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would returning
nullhelp with bootstrapping?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes entity null would mean entity not found and this would prompt bootstrap which inturn would run the bootstrap script. Otherwise RTE here is everything broken, i agree this is not an ideal but this is something we have from persistence we expect EntityResult
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this logic is too obscure IMHO. Would it be preferable to have a dedicated
isRealmBootstrapped(realmId)method?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about if take this is in my MetaStoreManager refactor ? #1462
I have assigned it to me