Introduce query history write buffer for DB resiliency #794

Peiyingy · 2025-11-11T00:37:15Z

Description

Following the work in #783 to introduce caching in HaGatewayManager, this PR adds a write buffer mechanism to QueryHistoryManager. With this change, if the database becomes unavailable, Trino-Gateway can continue to route queries using the cached Trino cluster data while temporarily storing query history records in the write buffer. When the database is available again, the buffered history entries will be flushed. This allows Trino-Gateway to avoid using the database as a single point of failure and improves overall resiliency.

This approach has already been implemented and validated in production at LinkedIn.

Testing

mvn clean install

cla-bot · 2025-11-11T00:37:18Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Peiying Ye.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email [email protected]
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

cla-bot · 2025-11-11T00:39:04Z

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Peiying Ye.
This is most likely caused by a git client misconfiguration; please make sure to:

check if your git client is configured with an email to sign commits git config --list | grep email
If not, set it up using git config --global user.email [email protected]
Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

trigger build

oneonestar · 2025-11-25T08:12:30Z

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

    {
        dao = requireNonNull(jdbi, "jdbi is null").onDemand(QueryHistoryDao.class);
        this.isOracleBackend = isOracleBackend;
+        if (writeBufferConfig != null && writeBufferConfig.isEnabled()) {


Suggested change

if (writeBufferConfig != null && writeBufferConfig.isEnabled()) {

if (writeBufferConfig.isEnabled()) {

writeBufferConfig should never be null. If it's null for whatever reason, we should fail fast.

oneonestar · 2025-11-25T08:34:54Z

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

+    public void buffer(T item)
+    {
+        if (!deque.offerLast(item)) {
+            deque.pollFirst();
+            deque.offerLast(item);
+        }
+    }


This needs synchronized to be thread safe.

oneonestar · 2025-11-26T08:34:24Z

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

+        int flushed = 0;
+        for (T next; (next = deque.pollFirst()) != null; ) {
+            try {
+                flusher.accept(next);
+                flushed++;
+            }
+            catch (RuntimeException e) {
+                deque.offerFirst(next);
+                break; // stop after first failure
+            }
+        }


This may fail to insert back to the queue when it's full.
Maybe peek() -> accept() -> remove() ?

Chaho12 · 2025-12-10T14:34:24Z

gateway-ha/src/main/java/io/trino/gateway/ha/config/WriteBufferConfiguration.java

+public class WriteBufferConfiguration
+{
+    private boolean enabled;
+    private int maxCapacity = 10000;


What is the reason for this number 10,000?

This is just a default value, same for the flushInterval. You can always overwrite them in the config file. I'll also update the default value settings in the doc. Do you recommend to set different default values for them?

I was just wondering what is the justification for this number 10,000. Maybe other projects use this number for buffer?

Chaho12 · 2025-12-10T14:34:53Z

gateway-ha/src/main/java/io/trino/gateway/ha/config/WriteBufferConfiguration.java

+{
+    private boolean enabled;
+    private int maxCapacity = 10000;
+    private Duration flushInterval = new Duration(2, TimeUnit.SECONDS);


Same for 2 seconds. What is the reason for this number?

See #794 (comment)

Chaho12 · 2025-12-10T14:40:12Z

gateway-ha/src/main/java/io/trino/gateway/ha/config/WriteBufferConfiguration.java

+
+    public void setMaxCapacity(int maxCapacity)
+    {
+        this.maxCapacity = maxCapacity;


Maybe check for negative/zero number?

Chaho12 · 2025-12-10T14:42:13Z

gateway-ha/src/main/java/io/trino/gateway/ha/config/WriteBufferConfiguration.java

+    public WriteBufferConfiguration() {}
+
+    public WriteBufferConfiguration(boolean enabled, int maxCapacity, Duration flushInterval)
+    {
+        this.enabled = enabled;
+        this.maxCapacity = maxCapacity;
+        this.flushInterval = flushInterval;
+    }


Why would you need two constructors?

Chaho12 · 2025-12-10T14:44:53Z

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

+        }
+        catch (RuntimeException e) {
+            if (isConnectionIssue(e) && writeBuffer != null) {
+                writeBuffer.buffer(queryDetail);


What happens if writer buffer is full?

Then we won't buffer this record and the query submission would fail - same behavior as the current code without this change.

Since this is an improvement to possible issue of loosing record, how about we at least log error that information is lost and increase buffer and/or change flush interval?

Chaho12 · 2025-12-10T14:50:02Z

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

+                    writeBufferConfig.getFlushInterval().toMillis(),
+                    writeBufferConfig.getFlushInterval().toMillis(),
+                    TimeUnit.MILLISECONDS);
+        }


Couldn't there be a race condition

Could you please elaborate on the race condition here? The flusher scheduledExecutor is a single thread here, and the buffering thread safe issue is handled in the WriteBuffer class

The single-threaded scheduler doesn't eliminate the race condition—it just means only one thread calls flushAll(), but that thread still races with multiple threads calling buffer().

Even though the scheduledExecutor is single-threaded, the race condition occurs between:

Multiple application threads calling buffer() (synchronized)

Single scheduler thread calling flushAll() (NOT synchronized)

Any thread calling size() (NOT synchronized)

https://github.com/trinodb/trino-gateway/pull/794/files#r2608879369 Check this part

Chaho12 · 2025-12-11T02:19:24Z

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

+    public int flushAll(Consumer<T> flusher)
+    {
+        int flushed = 0;
+        for (T next; (next = deque.peekFirst()) != null; ) {
+            try {
+                flusher.accept(next);
+            }
+            catch (RuntimeException e) {
+                break; // stop after first failure
+            }
+            // Only remove after a successful flush
+            deque.pollFirst();
+            flushed++;
+        }
+        return flushed;
+    }
+
+    public int size()
+    {
+        return deque.size();
+    }
+}


Suggested change

public int flushAll(Consumer<T> flusher)

{

int flushed = 0;

for (T next; (next = deque.peekFirst()) != null; ) {

try {

flusher.accept(next);

}

catch (RuntimeException e) {

break; // stop after first failure

}

// Only remove after a successful flush

deque.pollFirst();

flushed++;

}

return flushed;

}

public int size()

{

return deque.size();

}

}

public synchronized int flushAll(Consumer<T> flusher) // Add synchronized

{

int flushed = 0;

for (T next; (next = deque.peekFirst()) != null; ) {

try {

flusher.accept(next);

}

catch (RuntimeException e) {

break; // stop after first failure

}

// Only remove after a successful flush

deque.pollFirst();

flushed++;

}

return flushed;

}

public synchronized int size() // Add synchronized

{

return deque.size();

}

}

Peiyingy force-pushed the query-history-write-buffer branch from 0703b78 to 0512583 Compare November 11, 2025 00:39

Introduce query history write buffer for DB resiliency

8c52dd0

Peiyingy force-pushed the query-history-write-buffer branch from 0512583 to 8c52dd0 Compare November 11, 2025 00:40

cla-bot bot added the cla-signed label Nov 11, 2025

trigger build

caab5db

trigger build

Peiyingy force-pushed the query-history-write-buffer branch from 85776ef to caab5db Compare November 12, 2025 19:00

Peiyingy marked this pull request as ready for review November 12, 2025 19:32

oneonestar reviewed Nov 26, 2025

View reviewed changes

xkrogen mentioned this pull request Dec 4, 2025

Add New Table for Audit Logs #803

Open

Chaho12 reviewed Dec 10, 2025

View reviewed changes

address comments

26af119

Chaho12 reviewed Dec 11, 2025

View reviewed changes

	if (writeBufferConfig != null && writeBufferConfig.isEnabled()) {
	if (writeBufferConfig.isEnabled()) {

Introduce query history write buffer for DB resiliency #794

Are you sure you want to change the base?

Introduce query history write buffer for DB resiliency #794

Uh oh!

Conversation

Peiyingy commented Nov 11, 2025

Description

Testing

Uh oh!

cla-bot bot commented Nov 11, 2025

Uh oh!

cla-bot bot commented Nov 11, 2025

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Peiyingy Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

Peiyingy Dec 11, 2025 •

edited

Loading