From e2623d63d4b6fad0b12fb9ace842475e4a9134dc Mon Sep 17 00:00:00 2001
From: Vladimir Sitnikov <sitnikov.vladimir@gmail.com>
Date: Thu, 25 Oct 2018 10:11:28 +0300
Subject: [PATCH] perf: fix 1ms per async CopyAPI (regression since 42.2.5)
 (#1314)

The problem is SSL sockets use buffers for input, and `stream#available()` might easily return 0 even though the data is available in the downstream socket. The only viable WA is to perform `read` calls, however it might impact performance (e.g. for async copy operations). The resolution is to throttle read calls for copy api, and keep 1ms reads for replication api.

fixes #1312
---
 CHANGELOG.md                                      |  4 ++++
 docs/_posts/2018-08-27-42.2.5-release.md          |  3 +++
 .../main/java/org/postgresql/core/PGStream.java   | 15 +++++++++++++++
 .../java/org/postgresql/core/v3/CopyDualImpl.java |  6 +-----
 .../v3/replication/V3ReplicationProtocol.java     |  2 ++
 5 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index be40476bb4..a3f3865a94 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,8 +9,12 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 ### Added
 
 ### Fixed
+- Fixed async copy performance regression [PR 1314](https://github.com/pgjdbc/pgjdbc/pull/1314)
 
 ## [42.2.5] (2018-08-27)
+### Known issues
+- 1ms per async copy call [issue 1312](https://github.com/pgjdbc/pgjdbc/issues/1312)
+
 ### Changed
 - `ssl=true` implies `sslmode=verify-full`, that is it requires valid server certificate [cdeeaca4](https://github.com/pgjdbc/pgjdbc/commit/cdeeaca47dc3bc6f727c79a582c9e4123099526e)
 
diff --git a/docs/_posts/2018-08-27-42.2.5-release.md b/docs/_posts/2018-08-27-42.2.5-release.md
index 5467ee107c..7277388170 100644
--- a/docs/_posts/2018-08-27-42.2.5-release.md
+++ b/docs/_posts/2018-08-27-42.2.5-release.md
@@ -7,6 +7,9 @@ version: 42.2.5
 ---
 **Notable changes**
 
+### Known issues
+- 1ms per async copy call [issue 1312](https://github.com/pgjdbc/pgjdbc/issues/1312)
+
 ### Changed
 - `ssl=true` implies `sslmode=verify-full`, that is it requires valid server certificate [cdeeaca4](https://github.com/pgjdbc/pgjdbc/commit/cdeeaca47dc3bc6f727c79a582c9e4123099526e)
 
diff --git a/pgjdbc/src/main/java/org/postgresql/core/PGStream.java b/pgjdbc/src/main/java/org/postgresql/core/PGStream.java
index 6f0f3a2bea..907b3cf656 100644
--- a/pgjdbc/src/main/java/org/postgresql/core/PGStream.java
+++ b/pgjdbc/src/main/java/org/postgresql/core/PGStream.java
@@ -44,6 +44,11 @@ public class PGStream implements Closeable, Flushable {
   private OutputStream pg_output;
   private byte[] streamBuffer;
 
+  private long nextStreamAvailableCheckTime;
+  // This is a workaround for SSL sockets: sslInputStream.available() might return 0
+  // so we perform "1ms reads" once in a while
+  private int minStreamAvailableCheckDelay = 1000;
+
   private Encoding encoding;
   private Writer encodingWriter;
 
@@ -114,6 +119,12 @@ public boolean hasMessagePending() throws IOException {
       return true;
     }
     // In certain cases, available returns 0, yet there are bytes
+    long now = System.currentTimeMillis();
+    if (now < nextStreamAvailableCheckTime && minStreamAvailableCheckDelay != 0) {
+      // Do not use ".peek" too often
+      return false;
+    }
+    nextStreamAvailableCheckTime = now + minStreamAvailableCheckDelay;
     int soTimeout = getNetworkTimeout();
     setNetworkTimeout(1);
     try {
@@ -125,6 +136,10 @@ public boolean hasMessagePending() throws IOException {
     }
   }
 
+  public void setMinStreamAvailableCheckDelay(int delay) {
+    this.minStreamAvailableCheckDelay = delay;
+  }
+
   /**
    * Switch this stream to using a new socket. Any existing socket is <em>not</em> closed; it's
    * assumed that we are changing to a new socket that delegates to the original socket (e.g. SSL).
diff --git a/pgjdbc/src/main/java/org/postgresql/core/v3/CopyDualImpl.java b/pgjdbc/src/main/java/org/postgresql/core/v3/CopyDualImpl.java
index 8ff5cf88ea..076321f752 100644
--- a/pgjdbc/src/main/java/org/postgresql/core/v3/CopyDualImpl.java
+++ b/pgjdbc/src/main/java/org/postgresql/core/v3/CopyDualImpl.java
@@ -28,11 +28,7 @@ public long endCopy() throws SQLException {
   }
 
   public byte[] readFromCopy() throws SQLException {
-    if (received.isEmpty()) {
-      queryExecutor.readFromCopy(this, true);
-    }
-
-    return received.poll();
+    return readFromCopy(true);
   }
 
   @Override
diff --git a/pgjdbc/src/main/java/org/postgresql/core/v3/replication/V3ReplicationProtocol.java b/pgjdbc/src/main/java/org/postgresql/core/v3/replication/V3ReplicationProtocol.java
index 682374734c..c06fdf28a3 100644
--- a/pgjdbc/src/main/java/org/postgresql/core/v3/replication/V3ReplicationProtocol.java
+++ b/pgjdbc/src/main/java/org/postgresql/core/v3/replication/V3ReplicationProtocol.java
@@ -129,6 +129,8 @@ private void configureSocketTimeout(CommonOptions options) throws PSQLException
       }
 
       pgStream.getSocket().setSoTimeout(minimalTimeOut);
+      // Use blocking 1ms reads for `available()` checks
+      pgStream.setMinStreamAvailableCheckDelay(0);
     } catch (IOException ioe) {
       throw new PSQLException(GT.tr("The connection attempt failed."),
           PSQLState.CONNECTION_UNABLE_TO_CONNECT, ioe);