Replace H2 use for testing with PostgreSQL by Akanksha-kedia · Pull Request #719 · trinodb/trino-gateway

Akanksha-kedia · 2025-07-08T16:55:59Z

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.

Akanksha-kedia · 2025-07-08T16:56:18Z

@ebyhr

webapp/pnpm-lock.yaml

Akanksha-kedia · 2025-07-09T01:54:52Z

I ll have to remove that

…

On Wed, 9 Jul 2025 at 6:49 AM, Yuya Ebihara ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In webapp/pnpm-lock.yaml <#719 (comment)> : > @@ -1,2936 +0,0 @@ -lockfileVersion: '9.0' Is this change related to H2 removal? — Reply to this email directly, view it on GitHub <#719 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AVL2AZQUV6COWOFPLL5UJO33HRUZXAVCNFSM6AAAAACBBYEIISVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDSOJZGQ4DSNRYGY> . You are receiving this because you authored the thread.Message ID: ***@***.***>

ebyhr · 2025-07-09T09:53:58Z

I ll have to remove that

Why do we need to remove webapp/pnpm-lock.yaml?

Akanksha-kedia · 2025-07-09T10:02:02Z

I mean i ll remove that commit

…

On Wed, 9 Jul 2025 at 3:24 PM, Yuya Ebihara ***@***.***> wrote: *ebyhr* left a comment (trinodb/trino-gateway#719) <#719 (comment)> I ll have to remove that Why do we need to remove webapp/pnpm-lock.yaml? — Reply to this email directly, view it on GitHub <#719 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AVL2AZXSTFAUPT5SWPXZ7RT3HTREZAVCNFSM6AAAAACBBYEIISVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTANJRHE2TANZWGU> . You are receiving this because you authored the thread.Message ID: ***@***.***>

Akanksha-kedia · 2025-07-14T16:48:25Z

@ebyhr please review

gateway-ha/src/main/java/io/trino/gateway/ha/persistence/dao/ExactMatchSourceSelectorsDao.java

gateway-ha/src/test/java/io/trino/gateway/ha/TestingJdbcConnectionManager.java

gateway-ha/src/test/java/io/trino/gateway/ha/router/TestSpecificDbResourceGroupsManager.java

gateway-ha/src/test/java/io/trino/gateway/ha/router/TestResourceGroupsManager.java

Akanksha-kedia · 2025-07-16T10:28:17Z

@ebyhr have resolved please review.

Akanksha-kedia · 2025-07-16T10:51:03Z

Database Configuration Changes:
- In gateway-ha/config.yaml: Changed database connection parameters to use environment variables with defaults, making the configuration more flexible for different environments.
Timestamp Handling Improvements:
- In ExactMatchSourceSelectorsDao.java: Added a TimestampColumnMapper class to properly handle timestamp formatting.
- In HaResourceGroupsManager.java: Added automatic timestamp generation for updateTime when it's null or empty.
Test Framework Modernization:
- In TestingJdbcConnectionManager.java: Replaced H2 in-memory database with PostgreSQL using TestContainers.
- In TestSpecificDbResourceGroupsManager.java: Removed file-based setup and switched to TestContainers.
- In TestResourceGroupsManager.java: Updated test assertions to use newer Java methods (e.g., getFirst() instead of get(0)), and improved resource group ID lookup.

Akanksha-kedia · 2025-07-18T12:55:53Z

@mosabua please review.

Akanksha-kedia · 2025-07-21T15:16:38Z

@mosabua

Akanksha-kedia · 2025-07-25T06:13:03Z

Please review and help to merge
@ebyhr @mosabua

Akanksha-kedia · 2025-08-08T05:49:43Z

https://nexus-iq.visa.com/assets/index.html#/vulnerabilities/CVE-2021-42392

Akanksha-kedia · 2025-08-08T05:50:00Z

@mosabua @electrum @ebyhr please review

Akanksha-kedia · 2025-08-08T05:52:51Z

https://github.com/trinodb/trino-gateway/security/advisories/GHSA-hmf5-v5xc-323v#advisory-comment-134493

mosabua · 2025-09-18T20:26:48Z

We cut a new release now .. will hopefully be able to test soon .. can you rebase in the meantime

Akanksha-kedia · 2025-09-20T08:27:42Z

Sure thing!

…

On Fri, 19 Sep 2025 at 1:57 AM, Manfred Moser ***@***.***> wrote: *mosabua* left a comment (trinodb/trino-gateway#719) <#719 (comment)> We cut a new release now .. will hopefully be able to test soon .. can you rebase in the meantime — Reply to this email directly, view it on GitHub <#719 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AVL2AZW2ILDDCWBZ7RQEHGD3TMIR3AVCNFSM6AAAAACBBYEIISVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGMBZGU4DKMRQGE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

Akanksha-kedia · 2025-09-29T09:06:15Z

@mosabua please review

mosabua · 2025-10-11T23:09:08Z

With #780 merged now .. can you rebase and adjust and make sure CI passes?

Akanksha-kedia · 2025-10-12T11:31:08Z

Sure

…

On Sun, 12 Oct 2025 at 4:39 AM, Manfred Moser ***@***.***> wrote: *mosabua* left a comment (trinodb/trino-gateway#719) <#719 (comment)> With #780 <#780> merged now .. can you rebase and adjust and make sure CI passes? — Reply to this email directly, view it on GitHub <#719 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AVL2AZSW5GCHXLTFCR5C4ND3XGE2VAVCNFSM6AAAAACBBYEIISVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGOJTG4ZDGNZSGY> . You are receiving this because you authored the thread.Message ID: ***@***.***>

Akanksha-kedia · 2025-10-13T14:45:08Z

please review @mosabua

mosabua · 2026-01-10T19:17:40Z

Can you rebase and resolve conflicts here @Akanksha-kedia ? It would be good to get this in

mosabua

Overall I agree with the idea and PR. We definitely need to make sure all tests pass before merging.

One aspect to keep in mind also is that we will hopefully delete all the resource groups table stuff and the related code from Trino Gateway since it really is outside of the scope of the project as has been discussed in our team syncs before

mosabua · 2026-01-15T04:50:27Z

gateway-ha/src/test/java/io/trino/gateway/ha/router/TestSpecificDbResourceGroupsManager.java

-        Jdbi jdbi = HaGatewayProviderModule.createJdbi(db);
-        JdbcConnectionManager connectionManager = new JdbcConnectionManager(jdbi, db);
-        super.resourceGroupManager = new HaResourceGroupsManager(connectionManager);
+        PostgreSQLContainer<?> postgres = new PostgreSQLContainer<>("postgres:14-alpine")


why such an old version of PostgreSQL? In either case .. it should be same as in other tests and usage within the codebase

gateway-ha/src/test/java/io/trino/gateway/ha/persistence/TestJdbcConnectionManager.java

mosabua · 2026-01-22T05:39:00Z

Ping @Akanksha-kedia

Akanksha-kedia · 2026-01-22T06:24:41Z

Yes will try asap

…

On Thu, 22 Jan 2026 at 11:09 AM, Manfred Moser ***@***.***> wrote: *mosabua* left a comment (trinodb/trino-gateway#719) <#719 (comment)> Ping @Akanksha-kedia <https://github.com/Akanksha-kedia> — Reply to this email directly, view it on GitHub <#719 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AVL2AZVB2AW67DO2V3WVX734IBPAXAVCNFSM6AAAAACBBYEIISVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOOBSGU4DOOJWGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Akanksha-kedia · 2026-01-23T11:00:06Z

Overall I agree with the idea and PR. We definitely need to make sure all tests pass before merging.

One aspect to keep in mind also is that we will hopefully delete all the resource groups table stuff and the related code from Trino Gateway since it really is outside of the scope of the project as has been discussed in our team syncs before

by when or anyhelp needed from my end

ebyhr · 2025-07-21T04:19:15Z

gateway-ha/src/main/java/io/trino/gateway/ha/persistence/dao/ExactMatchSourceSelectorsDao.java

+                    return timestamp != null ? timestamp.toLocalDateTime().format(FORMATTER) : null;
+                }
+                catch (SQLException e) {
+                    return LocalDateTime.now(ZoneId.systemDefault()).format(FORMATTER);


Please leave a code comment why we need this fallback logic.

When does this issue happen? Is it covered by tests?

Based on my analysis, the tests do exercise the timestamp mapper (through testReadExactMatchSourceSelectors()), but they don't specifically test the fallback scenario where getTimestamp() throws a SQLException. The fallback is a defensive measure for edge cases with certain database drivers.

ebyhr · 2026-01-22T08:10:37Z

gateway-ha/src/test/java/io/trino/gateway/ha/router/TestResourceGroupsManager.java

-        assertThat(resourceGroups.get(0).getMaxQueued()).isEqualTo(200);
-        assertThat(resourceGroups.get(0).getJmxExport()).isTrue();
-        assertThat(resourceGroups.get(0).getSoftMemoryLimit()).isEqualTo("80%");
+        assertThat(resourceGroups.getFirst().getResourceGroupId()).isEqualTo(1L);


This change is unrelated to H2->PostgreSQL change. Please separate a commit.

ebyhr · 2026-01-22T08:10:52Z

gateway-ha/src/test/java/io/trino/gateway/ha/router/TestResourceGroupsManager.java

    {
+        List<ResourceGroupsDetail> resourceGroups = resourceGroupManager.readAllResourceGroups(null);
+        long adminResourceGroupId = resourceGroups.stream()
+                .filter(rg -> "admin".equals(rg.getName()))


rg -> resourceGroup
https://github.com/trinodb/trino/blob/master/.github/DEVELOPMENT.md#avoid-abbreviations

ebyhr · 2026-01-22T08:11:43Z

gateway-ha/src/test/java/io/trino/gateway/ha/router/TestResourceGroupsManager.java

+                .findFirst()
+                .orElse(null);
+
+        if (testGroup == null) {


Why is this test non-deterministic?

The test code in TestResourceGroupsManager.java at line 237-240 is non-deterministic because it uses .stream().filter().findFirst() on a List without guaranteed ordering:

ResourceGroupsDetail testGroup = existingGroups.stream() .filter(rg -> "selector-test-group".equals(rg.getName())) .findFirst() .orElse(null);

Root Cause:

Database ordering is not guaranteed: The readAllResourceGroups(null) method returns a List from a database query, but without an explicit ORDER BY clause, the database can return rows in any order.

Test execution order dependency: The test uses @TestMethodOrder(OrderAnnotation.class) and @Order annotations, but previous tests create multiple resource groups. The order in which these groups are returned from the database is unpredictable.

Stream.findFirst() behavior: When filtering a stream, findFirst() returns the first element that matches the filter predicate. If multiple elements could match (or if the list order changes), the result becomes non-deterministic.

The Problem:

If there are multiple resource groups in the database when this test runs, and the database returns them in different orders on different executions, the test behavior becomes unpredictable. Even though the filter looks for a specific name ("selector-test-group"), the order in which the database returns all groups can vary, affecting test execution timing and state.

Solution:

The code should either:

Use a deterministic query with explicit ordering in readAllResourceGroups()

Not rely on list ordering when searching for specific items

Clean up test data more reliably between test methods

The pattern appears in multiple places in the test file (lines 90-94, 237-240, 268-271, 305-310), making the entire test suite potentially flaky.

How does this change relate to the H2 → PostgreSQL migration?

ebyhr · 2026-01-22T08:13:50Z

gateway-ha/src/test/java/io/trino/gateway/ha/HaGatewayTestUtils.java

+        assumeTrue("true".equals(System.getenv("GITHUB_ACTIONS"))
+                || "x86_64".equalsIgnoreCase(System.getProperty("os.arch"))
+                || "true".equals(System.getenv("TG_RUN_ORACLE_TESTS")));
+        return new OracleContainer("gvenzl/oracle-xe:18.4.0-slim");


Why do you downgrade the version? Please revert this change.

Akanksha-kedia · 2026-01-23T11:34:02Z

please review @mosabua

ebyhr

Replacing H2 with PostgreSQL is totally fine. However, some test changes don't make sense to me.

ebyhr · 2026-01-23T23:15:07Z

gateway-ha/src/test/java/io/trino/gateway/ha/router/TestResourceGroupsManager.java

        assertThat(resourceGroups.get(0).getHardConcurrencyLimit()).isEqualTo(20);
        assertThat(resourceGroups.get(0).getMaxQueued()).isEqualTo(200);
-        assertThat(resourceGroups.get(0).getJmxExport()).isTrue();
+        assertThat(resourceGroups.get(0).getJmxExport()).isEqualTo(true);


ebyhr · 2026-01-23T23:22:14Z

gateway-ha/src/test/java/io/trino/gateway/ha/router/TestResourceGroupsManager.java

+        // Find the admin resource group and verify its properties
+        ResourceGroupsDetail adminGroup = resourceGroups.stream()
+                .filter(rg -> "admin".equals(rg.getName()))
+                .findFirst()


Please avoid findFirst as much as possible. Consider using collect(toOptional()) instead.

ebyhr · 2026-01-23T23:26:26Z

gateway-ha/src/main/java/io/trino/gateway/ha/router/HaResourceGroupsManager.java

            ExactSelectorsDetail exactSelectorDetail)
    {
+        // If updateTime is null or empty, set current timestamp
+        if (exactSelectorDetail.getUpdateTime() == null || exactSelectorDetail.getUpdateTime().trim().isEmpty()) {


getUpdateTime() contains @Nonnull annotation.

Change DateTimeFormatter formatter to a constant.

ebyhr · 2026-01-23T23:29:03Z

gateway-ha/src/test/java/io/trino/gateway/ha/router/TestSpecificDbResourceGroupsManager.java

        extends TestResourceGroupsManager
 {
-    private String specificDb;
+    private String specificDb = "test_db_specific";


Make it static final and uppercase the constant name.

ebyhr · 2026-01-23T23:29:38Z

gateway-ha/src/test/java/io/trino/gateway/ha/router/TestSpecificDbResourceGroupsManager.java

+        catch (Exception e) {
+            throw new RuntimeException("Failed to setup test database: " + specificDb, e);
+        }


Remove a redundant catch block.

ebyhr · 2026-01-23T23:29:58Z

gateway-ha/src/test/java/io/trino/gateway/ha/router/TestSpecificDbResourceGroupsManager.java

+        String jdbcUrl = postgres.getJdbcUrl();
+        String username = postgres.getUsername();
+        String password = postgres.getPassword();


ebyhr · 2026-01-23T23:30:26Z

gateway-ha/src/test/java/io/trino/gateway/ha/TestingJdbcConnectionManager.java

+        String jdbcUrl = postgres.getJdbcUrl();
+        String username = postgres.getUsername();
+        String password = postgres.getPassword();


ebyhr · 2026-01-23T23:31:11Z

gateway-ha/src/test/java/io/trino/gateway/ha/TestingJdbcConnectionManager.java

-import java.io.File;
-import java.nio.file.Path;
+import org.testcontainers.containers.JdbcDatabaseContainer;
+import org.testcontainers.containers.PostgreSQLContainer;


Please avoid a deprecated class.

ebyhr · 2026-01-23T23:32:32Z

gateway-ha/src/test/java/io/trino/gateway/ha/TestingJdbcConnectionManager.java

+                postgres.getJdbcUrl(),
+                postgres.getUsername(),
+                postgres.getPassword(),
+                "org.postgresql.Driver",


PostgreSQLContainer offers getDriverClassName method.

Akanksha-kedia · 2026-01-27T06:46:54Z

@ebyhr please review all and let me know wat more chnages i need to do

ebyhr · 2026-01-27T07:34:15Z

Please fix build failures:

Error:  src/test/java/io/trino/gateway/ha/router/TestResourceGroupsManager.java:[26,15] (imports) UnusedImports: Unused import - com.google.common.collect.MoreObjects.firstNonNull.
Error:  src/test/java/io/trino/gateway/ha/router/TestResourceGroupsManager.java:[32,15] (imports) UnusedImports: Unused import - java.util.stream.Collectors.collectingAndThen.
Error:  src/test/java/io/trino/gateway/ha/router/TestResourceGroupsManager.java:[33,15] (imports) UnusedImports: Unused import - java.util.stream.Collectors.toList.

Akanksha-kedia · 2026-01-27T09:46:13Z

Please fix build failures:

Error:  src/test/java/io/trino/gateway/ha/router/TestResourceGroupsManager.java:[26,15] (imports) UnusedImports: Unused import - com.google.common.collect.MoreObjects.firstNonNull.
Error:  src/test/java/io/trino/gateway/ha/router/TestResourceGroupsManager.java:[32,15] (imports) UnusedImports: Unused import - java.util.stream.Collectors.collectingAndThen.
Error:  src/test/java/io/trino/gateway/ha/router/TestResourceGroupsManager.java:[33,15] (imports) UnusedImports: Unused import - java.util.stream.Collectors.toList.

done @ebyhr do review its a long pending pr thanks in advance.

Akanksha-kedia · 2026-01-27T10:33:18Z

@mosabua please help to review and merge or else again we have spend time in rebasing and all

ebyhr · 2026-01-27T22:54:28Z

gateway-ha/src/test/java/io/trino/gateway/ha/TestingJdbcConnectionManager.java


+    public static JdbcConnectionManager createTestingJdbcConnectionManager()
+    {
+        PostgreSQLContainer postgres = new PostgreSQLContainer("postgres:14-alpine")


Why not use TestcontainersUtils.createPostgreSqlContainer?

github-actions · 2026-02-18T17:23:30Z

This pull request has gone a while without any activity. Ask for help on #trino-gateway-dev on Trino slack.

mosabua · 2026-02-18T20:49:24Z

We definitely want to go ahead with this so I am adding the stale-ignore label to keep the PR from getting closed/lost.

cla-bot bot added the cla-signed label Jul 8, 2025

ebyhr reviewed Jul 9, 2025

View reviewed changes

webapp/pnpm-lock.yaml Show resolved Hide resolved

Akanksha-kedia force-pushed the feature/new_pr branch 2 times, most recently from d8bbdb2 to c1469cc Compare July 9, 2025 02:23

Akanksha-kedia force-pushed the feature/new_pr branch from c1469cc to 414ca10 Compare July 14, 2025 14:45

ebyhr reviewed Jul 15, 2025

View reviewed changes

Akanksha-kedia force-pushed the feature/new_pr branch from 414ca10 to 52aaeaa Compare July 16, 2025 08:56

Akanksha-kedia force-pushed the feature/new_pr branch from 52aaeaa to d505b71 Compare July 16, 2025 10:44

Akanksha-kedia force-pushed the feature/new_pr branch from d505b71 to 11a5dd7 Compare September 29, 2025 09:04

Akanksha-kedia force-pushed the feature/new_pr branch from 02e1226 to 48fc01b Compare October 13, 2025 10:50

mosabua reviewed Jan 15, 2026

View reviewed changes

Akanksha-kedia force-pushed the feature/new_pr branch 2 times, most recently from 3c528b2 to 25982c4 Compare January 23, 2026 11:17

ebyhr reviewed Jan 23, 2026

View reviewed changes

Akanksha-kedia force-pushed the feature/new_pr branch from 25982c4 to fdd7243 Compare January 23, 2026 11:29

Akanksha-kedia force-pushed the feature/new_pr branch 4 times, most recently from c155698 to 891a4ba Compare January 23, 2026 11:54

ebyhr requested changes Jan 23, 2026

View reviewed changes

Akanksha-kedia force-pushed the feature/new_pr branch from 891a4ba to dafca77 Compare January 27, 2026 06:45

Akanksha-kedia force-pushed the feature/new_pr branch 3 times, most recently from 31386a1 to 146e761 Compare January 27, 2026 09:45

Replace H2 use for testing with PostgreSQL

e521905

Akanksha-kedia force-pushed the feature/new_pr branch from 146e761 to e521905 Compare January 27, 2026 10:12

ebyhr reviewed Jan 27, 2026

View reviewed changes

github-actions bot added the stale label Feb 18, 2026

mosabua added stale-ignore Use this label on PRs that should be ignored by the stale bot so they are not flagged or closed. and removed stale labels Feb 18, 2026

Conversation

Akanksha-kedia commented Jul 8, 2025 • edited by ebyhr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release notes

Uh oh!

Akanksha-kedia commented Jul 8, 2025

Uh oh!

Uh oh!

Akanksha-kedia commented Jul 9, 2025 via email

Uh oh!

ebyhr commented Jul 9, 2025

Uh oh!

Akanksha-kedia commented Jul 9, 2025 via email

Uh oh!

Akanksha-kedia commented Jul 14, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Akanksha-kedia commented Jul 16, 2025

Uh oh!

Akanksha-kedia commented Jul 16, 2025

Uh oh!

Akanksha-kedia commented Jul 18, 2025

Uh oh!

Akanksha-kedia commented Jul 21, 2025

Uh oh!

Akanksha-kedia commented Jul 25, 2025

Uh oh!

Akanksha-kedia commented Aug 8, 2025

Uh oh!

Akanksha-kedia commented Aug 8, 2025

Uh oh!

Akanksha-kedia commented Aug 8, 2025

Uh oh!

mosabua commented Sep 18, 2025

Uh oh!

Akanksha-kedia commented Sep 20, 2025 via email

Uh oh!

Akanksha-kedia commented Sep 29, 2025

Uh oh!

mosabua commented Oct 11, 2025

Uh oh!

Akanksha-kedia commented Oct 12, 2025 via email

Uh oh!

Akanksha-kedia commented Oct 13, 2025

Uh oh!

mosabua commented Jan 10, 2026

Uh oh!

mosabua left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mosabua commented Jan 22, 2026

Uh oh!

Akanksha-kedia commented Jan 22, 2026 via email

Uh oh!

Akanksha-kedia commented Jan 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Akanksha-kedia commented Jul 8, 2025 •

edited by ebyhr

Loading

ebyhr left a comment •

edited

Loading