Add the doc on how to configure Postgres as the backend database #296

aihuaxu · 2024-09-16T05:54:23Z

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change.

This is to document how to configure metastore through EclipseLink for H2 database and Postgres.

H2 dependency is removed from the build and the user can explicitly add database dependencies during the build.

Fixes #230

Type of change

Please delete options that are not relevant.

Documentation update
New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

This has been tested through gradle build and by switching to Postgres database.
Test B

Test Configuration:

Hardware:
Toolchain:
SDK:

Checklist:

Please delete options that are not relevant.

I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
If adding new functionality, I have discussed my implementation with the community using the linked GitHub issue

sfc-gh-aixu · 2024-09-16T16:20:17Z

@flyrain, @MonkeyCanCode and @loicalleyne, can you help take a look?

MonkeyCanCode · 2024-09-16T16:29:39Z

@flyrain, @MonkeyCanCode and @loicalleyne, can you help take a look?

Thanks for adding this doc. I will review it later tonight.

MonkeyCanCode · 2024-09-17T01:49:51Z

polaris-service/build.gradle.kts

@@ -111,7 +111,6 @@ dependencies {
  testRuntimeOnly("org.junit.platform:junit-platform-launcher")

  testRuntimeOnly(project(":polaris-eclipselink"))
-  testRuntimeOnly(libs.h2)


I think this one is needed for test.

Actually the one in extension/persistence/eclipselink/build.gradle.kts is needed to test out PolarisEclipseLinkMetaStoreManagerTest. This one is needed if we want to run integration test against eclipselink in polaris-service.

For now, we are not running the integration tests against eclipseLink. We can manually run with -PeclipseLink=true -PeclipseLinkDeps=com.h2database:h2:2.3.232. So we can remove this.

MonkeyCanCode · 2024-09-17T01:54:18Z

docs/metastores.md

+```
+Build with Postgres dependency and run Polaris service:
+> polaris> ./gradlew --no-daemon --info -PeclipseLink=true -PeclipseLinkDeps=org.postgresql:postgresql:42.7.4 clean shadowJar
+> polaris> java -jar  polaris-service/build/libs/polaris-service-*.jar server ./polaris-server.yml


NIT: add one more block quotes (">") here so they will show in two lines instead of one line

MonkeyCanCode · 2024-09-17T01:54:25Z

docs/metastores.md

+```
+
+Build with H2 dependency and run Polaris service:
+> polaris> ./gradlew --no-daemon --info -PeclipseLink=true -PeclipseLinkDeps=com.h2database:h2:2.3.232 clean shadowJar


NIT: add one more block quotes (">") here so they will show in two lines instead of one line

MonkeyCanCode · 2024-09-17T01:57:13Z

docs/metastores.md

+The following shows a sample configuration for Postgres database.
+
+```angular2html
+  <persistence-unit name="polaris" transaction-type="RESOURCE_LOCAL">


NIT: remove 2 empty leading spaces on line 75

MonkeyCanCode · 2024-09-17T01:58:05Z

docs/metastores.md

+  <shared-cache-mode>NONE</shared-cache-mode>
+  <properties>
+    <property name="jakarta.persistence.jdbc.url"
+              value="jdbc:postgresql://localhost:5432/{realm}"/>


So from my local testing, I noticed I can set this to any database name and it will still work (using a simple realm in this case which is the default realm). Do we want to add example on how to use multi-realms?

Since we are using one realm, it doesn't conflict. This is the example to configure for multi-realms, in which case, each realm is mapped to different database. Are you saying to show examples on how to use multi-realms for Polaris in general? Let me add that in a follow up.

Yes, I was looking for that earlier but didn't find much. Haven't tried out the multi realms setup as of now.

MonkeyCanCode · 2024-09-17T01:58:47Z

docs/metastores.md

+      <property name="jakarta.persistence.jdbc.password" value=""/>
+      <property name="jakarta.persistence.schema-generation.database.action" value="create"/>
+    </properties>
+  </persistence-unit>


NIT: remove 2 empty leading spaces on line 63

flyrain

LGTM with minor suggestions. Thanks @aihuaxu!

flyrain · 2024-09-17T18:07:27Z

docs/configuring-polaris-for-production.md

-To use [EclipseLink](https://eclipse.dev/eclipselink/) for metastore management, specify the configuration `metaStoreManager.conf-file` to point to an [EclipseLink `persistence.xml` file](https://eclipse.dev/eclipselink/documentation/2.5/solutions/testingjpa002.htm). This file, local to the Polaris service, will contain information on what database to use for metastore management and how to connect to it.
+To use [EclipseLink](https://eclipse.dev/eclipselink/) for metastore management, specify the configuration `metaStoreManager.conf-file` to point to an [EclipseLink `persistence.xml` file](https://eclipse.dev/eclipselink/documentation/4.0/solutions/solutions.html#TESTINGJPA002). This file, local to the Polaris service, will contain information on what database to use for metastore management and how to connect to it. Check out [metastore doc](./metastores.md) for details.


I'd remove the paragraph completely, and only keep a link to metastores.md, or move everything from metastores.md to here. I'd prefer the later to avoid jumping between two pages for readers. I admit this will make the page a bit longer, but I think it's fine comparing to jump between two pages while following instructions. WDYT?

I feel it makes the page too long since we may add more sample configuration like mysql. I would prefer to simplify this paragraph to have a summary here and add a link. How do you think?

I think it's important to complete the setting by just looking at one page if we really care about how easy developer can setup a production ready Polaris. We can remove unnecessary stuff to make it compact, like putting the MySQL and H2 to separated doc, and only keep Postgres here. With that, it isn't a blocker for this PR. We can always improve on it.

Sounds good. I will keep them separate for now. Feel it would be strange to have Postgres in the main doc and have the others separately. But it's not critical and we can merge later if needed.

flyrain · 2024-09-17T18:20:57Z

docs/metastores.md

+```
+`conf-file` points to the EclipseLink configuration file and `persistence-unit` tells which unit from the configuration file to use for database connection.
+
+E.g., `polaris-dev` and `polaris` persistence units are configured in the same configuration file to connect to development database and production database respectively. Updating `persistence-unit` from `polaris-dev` to `polaris` switch from the development to the production.


We would say persistence.xml instead of the same configuration file, at least that's how I got confused in the beginning; is it polaris-server.yml or persistence.xml when we are talking about the the same configuration file?

Let me update that to make it clear.

flyrain · 2024-09-17T18:26:36Z

docs/metastores.md

+2) Use `conf-file: /tmp/conf.jar!/persistence.xml`.
+
+## EclipseLink Configuration - persistence.xml
+Configuration file `persistence.xml` is to configure the database connection properties, and the properties vary based on different databases and database setup.


How about this?

The persistence.xml file configures database connection properties, which vary based on the database and its setup.

Rephrase to "The configuration file persistence.xml is used to set up the database connection properties, which can differ depending on the type of database and its configuration.". How do you think?

Fine with that. BTW, is it allowed to have a different config file name other than persistence.xml?

Yes. Different file names are allowed but EclipseLink doc always uses persistence.xml (https://eclipse.dev/eclipselink/documentation/2.5/solutions/testingjpa002.htm).

docs/metastores.md

flyrain · 2024-09-17T20:40:36Z

docs/configuring-polaris-for-production.md

@@ -56,7 +56,7 @@ Be sure to secure your metastore backend since it will be storing credentials an

 ### Configuring EclipseLink

-To use [EclipseLink](https://eclipse.dev/eclipselink/) for metastore management, specify the configuration `metaStoreManager.conf-file` to point to an [EclipseLink `persistence.xml` file](https://eclipse.dev/eclipselink/documentation/2.5/solutions/testingjpa002.htm). This file, local to the Polaris service, will contain information on what database to use for metastore management and how to connect to it.
+To use [EclipseLink](https://eclipse.dev/eclipselink/) for metastore management, specify the configuration `metaStoreManager.conf-file` to point to an [EclipseLink `persistence.xml` file](https://eclipse.dev/eclipselink/documentation/4.0/solutions/solutions.html#TESTINGJPA002). This file, local to the Polaris service, contains details of the database used for metastore management and the connection settings. For more information, refer to [metastore documentation](./metastores.md) for details.


I don't think people will understand the summary, especially for the first-time reader. I feel It even causes more confusion. How about just this?

Check [metastore documentation](./metastores.md) for instructions.

I think we need to provide some summary here to give some high level information to align with the rest as I read this doc and then they can look at the separate page if needed to understand more?

The point is that developer will always have to click into the detailed page for instructions. There is a rare chance that they can do anything by just looking at the summary. Moreover, we provided 3 links in the summary here, there is a good chance that reader will click first two links, but still lost. What they really need is the third link(./metastores.md).

That makes sense. I will remove the first two links so the users can check out the separate page metastores.md if that causes confusion.

flyrain · 2024-09-17T23:27:04Z

Thanks @aihuaxu for the PR. Thanks @MonkeyCanCode for the review.

aihuaxu requested review from jbonofre, ashvina, RussellSpitzer, snazy, vvcephei, takidau, jackye1995 and flyrain as code owners September 16, 2024 05:54

MonkeyCanCode reviewed Sep 17, 2024

View reviewed changes

aihuaxu added 3 commits September 16, 2024 20:30

Add the doc on how to configure Postgres as the backend database

a26a9be

Add a link to metastores.md

af3c523

Address comments

4e7a42c

aihuaxu force-pushed the aixu-postgres-doc branch from 717160d to 4e7a42c Compare September 17, 2024 04:01

aihuaxu requested a review from MonkeyCanCode September 17, 2024 04:01

flyrain reviewed Sep 17, 2024

View reviewed changes

Update documents

44d0626

aihuaxu force-pushed the aixu-postgres-doc branch from cb1b505 to 44d0626 Compare September 17, 2024 20:27

flyrain reviewed Sep 17, 2024

View reviewed changes

Update the doc

2b6f60c

flyrain approved these changes Sep 17, 2024

View reviewed changes

flyrain merged commit f374ab4 into apache:main Sep 17, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the doc on how to configure Postgres as the backend database #296

Add the doc on how to configure Postgres as the backend database #296

aihuaxu commented Sep 16, 2024 •

edited

Loading

sfc-gh-aixu commented Sep 16, 2024

MonkeyCanCode commented Sep 16, 2024

MonkeyCanCode Sep 17, 2024

aihuaxu Sep 17, 2024

MonkeyCanCode Sep 17, 2024

MonkeyCanCode Sep 17, 2024

MonkeyCanCode Sep 17, 2024

MonkeyCanCode Sep 17, 2024

aihuaxu Sep 17, 2024

MonkeyCanCode Sep 17, 2024

MonkeyCanCode Sep 17, 2024

flyrain left a comment

flyrain Sep 17, 2024

sfc-gh-aixu Sep 17, 2024

flyrain Sep 17, 2024

aihuaxu Sep 17, 2024

flyrain Sep 17, 2024 •

edited

Loading

sfc-gh-aixu Sep 17, 2024

flyrain Sep 17, 2024

aihuaxu Sep 17, 2024

flyrain Sep 17, 2024

aihuaxu Sep 17, 2024

flyrain Sep 17, 2024

sfc-gh-aixu Sep 17, 2024 •

edited

Loading

flyrain Sep 17, 2024

aihuaxu Sep 17, 2024 •

edited

Loading

flyrain commented Sep 17, 2024

		To use [EclipseLink](https://eclipse.dev/eclipselink/) for metastore management, specify the configuration `metaStoreManager.conf-file` to point to an [EclipseLink `persistence.xml` file](https://eclipse.dev/eclipselink/documentation/2.5/solutions/testingjpa002.htm). This file, local to the Polaris service, will contain information on what database to use for metastore management and how to connect to it.
		To use [EclipseLink](https://eclipse.dev/eclipselink/) for metastore management, specify the configuration `metaStoreManager.conf-file` to point to an [EclipseLink `persistence.xml` file](https://eclipse.dev/eclipselink/documentation/4.0/solutions/solutions.html#TESTINGJPA002). This file, local to the Polaris service, will contain information on what database to use for metastore management and how to connect to it. Check out [metastore doc](./metastores.md) for details.

Add the doc on how to configure Postgres as the backend database #296

Add the doc on how to configure Postgres as the backend database #296

Conversation

aihuaxu commented Sep 16, 2024 • edited Loading

Description

Type of change

How Has This Been Tested?

Checklist:

sfc-gh-aixu commented Sep 16, 2024

MonkeyCanCode commented Sep 16, 2024

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

flyrain left a 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

flyrain Sep 17, 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

sfc-gh-aixu Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aihuaxu Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

flyrain commented Sep 17, 2024

aihuaxu commented Sep 16, 2024 •

edited

Loading

flyrain Sep 17, 2024 •

edited

Loading

sfc-gh-aixu Sep 17, 2024 •

edited

Loading

aihuaxu Sep 17, 2024 •

edited

Loading