From cf0c5681ab45921fcb16a8fdb3745d4155a9c38c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Tue, 14 Jun 2022 16:37:37 +0200 Subject: [PATCH 01/25] chore: refactor ProxyServer to use channels instead of sockets Refactors the ProxyServer to use the SocketChannel interfaces instead of Sockets. This makes the sockets replaceable for the new Unix domain socket implementation in Java16, which again means that we can conditionally load that when we know that we have Java16 or higher as the target language. --- pom.xml | 53 +++++++++ .../spanner/pgadapter/ConnectionHandler.java | 46 +++++--- .../pgadapter/Java17ServerSocketFactory.java | 65 +++++++++++ .../pgadapter/Java8ServerSocketFactory.java | 50 ++++++++ .../cloud/spanner/pgadapter/ProxyServer.java | 108 ++++++++++++------ .../pgadapter/ServerSocketFactory.java | 26 +++++ .../pgadapter/ConnectionHandlerTest.java | 25 ++-- 7 files changed, 302 insertions(+), 71 deletions(-) create mode 100644 src/main/java/com/google/cloud/spanner/pgadapter/Java17ServerSocketFactory.java create mode 100644 src/main/java/com/google/cloud/spanner/pgadapter/Java8ServerSocketFactory.java create mode 100644 src/main/java/com/google/cloud/spanner/pgadapter/ServerSocketFactory.java diff --git a/pom.xml b/pom.xml index 93ba7379f..829b5444a 100644 --- a/pom.xml +++ b/pom.xml @@ -275,6 +275,50 @@ + + native-image + + + + com.kohlschutter.junixsocket + junixsocket-core + 2.4.0 + pom + test + + + com.kohlschutter.junixsocket + junixsocket-common + 2.4.0 + test + + + + + + org.apache.maven.plugins + maven-compiler-plugin + + + com/google/cloud/spanner/pgadapter/Java8ServerSocketFactory.java + + 17 + 17 + + + + org.codehaus.mojo + animal-sniffer-maven-plugin + + true + + + + + @@ -299,6 +343,15 @@ + + org.apache.maven.plugins + maven-compiler-plugin + + + com/google/cloud/spanner/pgadapter/Java17ServerSocketFactory.java + + + org.apache.maven.plugins maven-surefire-plugin diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/ConnectionHandler.java b/src/main/java/com/google/cloud/spanner/pgadapter/ConnectionHandler.java index c22603c80..0497964ac 100644 --- a/src/main/java/com/google/cloud/spanner/pgadapter/ConnectionHandler.java +++ b/src/main/java/com/google/cloud/spanner/pgadapter/ConnectionHandler.java @@ -42,7 +42,10 @@ import java.io.DataOutputStream; import java.io.EOFException; import java.io.IOException; -import java.net.Socket; +import java.net.InetSocketAddress; +import java.nio.channels.ByteChannel; +import java.nio.channels.Channels; +import java.nio.channels.SocketChannel; import java.security.SecureRandom; import java.text.MessageFormat; import java.util.HashMap; @@ -72,7 +75,9 @@ public class ConnectionHandler extends Thread { private static final String CHANNEL_PROVIDER_PROPERTY = "CHANNEL_PROVIDER"; private final ProxyServer server; - private final Socket socket; + private final ByteChannel socketChannel; + private final InetSocketAddress remoteAddress; + private final String remoteClient; private final Map statementsMap = new HashMap<>(); private final Map portalsMap = new HashMap<>(); private static final Map activeStatementsMap = @@ -90,18 +95,26 @@ public class ConnectionHandler extends Thread { private WellKnownClient wellKnownClient; private ExtendedQueryProtocolHandler extendedQueryProtocolHandler; - ConnectionHandler(ProxyServer server, Socket socket) { + ConnectionHandler(ProxyServer server, ByteChannel byteChannel) throws IOException { super("ConnectionHandler-" + CONNECTION_HANDLER_ID_GENERATOR.incrementAndGet()); this.server = server; - this.socket = socket; + this.socketChannel = byteChannel; + if (byteChannel instanceof SocketChannel + && ((SocketChannel) byteChannel).getRemoteAddress() instanceof InetSocketAddress) { + SocketChannel socketChannel = (SocketChannel) byteChannel; + this.remoteAddress = (InetSocketAddress) socketChannel.getRemoteAddress(); + this.remoteClient = remoteAddress.getAddress().getHostAddress(); + } else { + this.remoteAddress = null; + this.remoteClient = "(local)"; + } this.secret = new SecureRandom().nextInt(); setDaemon(true); logger.log( Level.INFO, () -> String.format( - "Connection handler with ID %s created for client %s", - getName(), socket.getInetAddress().getHostAddress())); + "Connection handler with ID %s created for client %s", getName(), remoteClient)); } @InternalApi @@ -185,16 +198,17 @@ public void run() { Level.INFO, () -> String.format( - "Connection handler with ID %s starting for client %s", - getName(), socket.getInetAddress().getHostAddress())); + "Connection handler with ID %s starting for client %s", getName(), remoteClient)); try (DataInputStream input = - new DataInputStream(new BufferedInputStream(this.socket.getInputStream())); + new DataInputStream(new BufferedInputStream(Channels.newInputStream(socketChannel))); DataOutputStream output = - new DataOutputStream(new BufferedOutputStream(this.socket.getOutputStream()))) { + new DataOutputStream( + new BufferedOutputStream(Channels.newOutputStream(socketChannel)))) { if (!server.getOptions().disableLocalhostCheck() - && !this.socket.getInetAddress().isAnyLocalAddress() - && !this.socket.getInetAddress().isLoopbackAddress()) { + && this.remoteAddress != null + && !this.remoteAddress.getAddress().isAnyLocalAddress() + && !this.remoteAddress.getAddress().isLoopbackAddress()) { handleError( output, new IllegalAccessException("This proxy may only be accessed from localhost.")); return; @@ -237,7 +251,7 @@ public void run() { () -> String.format( "Exception on connection handler with ID %s for client %s: %s", - getName(), socket.getInetAddress().getHostAddress(), e)); + getName(), remoteClient, e)); } finally { logger.log( Level.INFO, () -> String.format("Closing connection handler with ID %s", getName())); @@ -245,7 +259,7 @@ public void run() { if (this.spannerConnection != null) { this.spannerConnection.close(); } - this.socket.close(); + this.socketChannel.close(); } catch (SpannerException | IOException e) { logger.log( Level.WARNING, @@ -276,9 +290,7 @@ void terminate() { if (this.status != ConnectionStatus.TERMINATED) { handleTerminate(); try { - if (!socket.isClosed()) { - socket.close(); - } + socketChannel.close(); } catch (IOException exception) { logger.log( Level.WARNING, diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/Java17ServerSocketFactory.java b/src/main/java/com/google/cloud/spanner/pgadapter/Java17ServerSocketFactory.java new file mode 100644 index 000000000..38a6c0a7f --- /dev/null +++ b/src/main/java/com/google/cloud/spanner/pgadapter/Java17ServerSocketFactory.java @@ -0,0 +1,65 @@ +// Copyright 2022 Google LLC +// +// Licensed 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 com.google.cloud.spanner.pgadapter; + +import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata; +import java.io.File; +import java.io.IOException; +import java.net.InetSocketAddress; +import java.net.StandardProtocolFamily; +import java.net.UnixDomainSocketAddress; +import java.nio.channels.ServerSocketChannel; +import java.nio.file.Path; + +// If your IDE is complaining about this file not being compatible with the language level +// of the project, then that is an indication that the IDE did not pick up the exclusion of this +// file that is defined in the pom.xml. +// This is a known issue in IntelliJ: https://youtrack.jetbrains.com/issue/IDEA-87868 +// +// Follow these steps to make IntelliJ ignore this compilation error: +// 1. Go to Settings. +// 2. Select Build, Execution, Deployment| Compiler | Excludes +// 3. Add this file to the list of excludes. +// See also https://www.jetbrains.com/help/idea/specifying-compilation-settings.html#5a737cfc + +class Java17ServerSocketFactory implements ServerSocketFactory { + + public Java17ServerSocketFactory() {} + + public ServerSocketChannel createTcpServerSocketChannel(OptionsMetadata options) + throws IOException { + ServerSocketChannel channel = ServerSocketChannel.open(); + InetSocketAddress address = new InetSocketAddress(options.getProxyPort()); + channel.configureBlocking(true); + channel.bind(address, options.getMaxBacklog()); + + return channel; + } + + public ServerSocketChannel creatUnixDomainSocketChannel(OptionsMetadata options, int localPort) + throws IOException { + File tempDir = new File(options.getSocketFile(localPort)); + Path path = Path.of(tempDir.toURI()); + if (tempDir.getParentFile() != null && !tempDir.getParentFile().exists()) { + tempDir.mkdirs(); + } + UnixDomainSocketAddress address = UnixDomainSocketAddress.of(path); + ServerSocketChannel domainSocketChannel = ServerSocketChannel.open(StandardProtocolFamily.UNIX); + domainSocketChannel.configureBlocking(true); + domainSocketChannel.bind(address, options.getMaxBacklog()); + + return domainSocketChannel; + } +} diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/Java8ServerSocketFactory.java b/src/main/java/com/google/cloud/spanner/pgadapter/Java8ServerSocketFactory.java new file mode 100644 index 000000000..4f2a3670a --- /dev/null +++ b/src/main/java/com/google/cloud/spanner/pgadapter/Java8ServerSocketFactory.java @@ -0,0 +1,50 @@ +// Copyright 2022 Google LLC +// +// Licensed 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 com.google.cloud.spanner.pgadapter; + +import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata; +import java.io.File; +import java.io.IOException; +import java.net.InetSocketAddress; +import java.nio.channels.ServerSocketChannel; +import org.newsclub.net.unix.AFUNIXServerSocketChannel; +import org.newsclub.net.unix.AFUNIXSocketAddress; + +class Java8ServerSocketFactory implements ServerSocketFactory { + + public Java8ServerSocketFactory() {} + + public ServerSocketChannel createTcpServerSocketChannel(OptionsMetadata options) + throws IOException { + ServerSocketChannel channel = ServerSocketChannel.open(); + InetSocketAddress address = new InetSocketAddress(options.getProxyPort()); + channel.configureBlocking(true); + channel.bind(address, options.getMaxBacklog()); + + return channel; + } + + public ServerSocketChannel creatUnixDomainSocketChannel(OptionsMetadata options, int localPort) + throws IOException { + File tempDir = new File(options.getSocketFile(localPort)); + if (tempDir.getParentFile() != null && !tempDir.getParentFile().exists()) { + tempDir.mkdirs(); + } + AFUNIXServerSocketChannel channel = AFUNIXServerSocketChannel.open(); + channel.bind(AFUNIXSocketAddress.of(tempDir), options.getMaxBacklog()); + + return channel; + } +} diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/ProxyServer.java b/src/main/java/com/google/cloud/spanner/pgadapter/ProxyServer.java index 1a00f5cc8..45a956251 100644 --- a/src/main/java/com/google/cloud/spanner/pgadapter/ProxyServer.java +++ b/src/main/java/com/google/cloud/spanner/pgadapter/ProxyServer.java @@ -28,11 +28,12 @@ import com.google.common.collect.ImmutableList; import java.io.BufferedOutputStream; import java.io.DataOutputStream; -import java.io.File; import java.io.IOException; -import java.net.ServerSocket; -import java.net.Socket; +import java.net.InetSocketAddress; import java.net.SocketException; +import java.nio.channels.Channels; +import java.nio.channels.ServerSocketChannel; +import java.nio.channels.SocketChannel; import java.util.Collections; import java.util.LinkedList; import java.util.List; @@ -44,8 +45,6 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Level; import java.util.logging.Logger; -import org.newsclub.net.unix.AFUNIXServerSocket; -import org.newsclub.net.unix.AFUNIXSocketAddress; /** * The proxy server listens for incoming client connections and starts a new {@link @@ -56,8 +55,28 @@ public class ProxyServer extends AbstractApiService { private static final Logger logger = Logger.getLogger(ProxyServer.class.getName()); private final OptionsMetadata options; private final Properties properties; + private final ServerSocketFactory serverSocketFactory = createServerSocketFactory(); private final List handlers = Collections.synchronizedList(new LinkedList<>()); + static ServerSocketFactory createServerSocketFactory() { + try { + return (ServerSocketFactory) + Class.forName("com.google.cloud.spanner.pgadapter.Java17ServerSocketFactory") + .getConstructor() + .newInstance(); + } catch (Exception ignore) { + } + try { + return (ServerSocketFactory) + Class.forName("com.google.cloud.spanner.pgadapter.Java8ServerSocketFactory") + .getConstructor() + .newInstance(); + } catch (Exception ignore) { + } + throw SpannerExceptionFactory.newSpannerException( + ErrorCode.FAILED_PRECONDITION, "No socket factory found!"); + } + /** * Latch that is closed when the TCP server has started. We need this to know the exact port that * the TCP socket was assigned, so we can assign the same port number to the Unix domain socket. @@ -68,7 +87,8 @@ public class ProxyServer extends AbstractApiService { * optionally one Unix domain socket, but could in theory be expanded to contain multiple sockets * of each type. */ - private final List serverSockets = Collections.synchronizedList(new LinkedList<>()); + private final List serverSocketChannels = + Collections.synchronizedList(new LinkedList<>()); private int localPort; @@ -170,18 +190,26 @@ public void run() { @Override protected void doStop() { - for (ServerSocket serverSocket : this.serverSockets) { + for (ServerSocketChannel serverSocketChannel : this.serverSocketChannels) { try { logger.log( - Level.INFO, () -> String.format("Server on socket %s is stopping", serverSocket)); - serverSocket.close(); + Level.INFO, + () -> + String.format( + "Server on socket channel %s is stopping", toString(serverSocketChannel))); + serverSocketChannel.close(); logger.log( - Level.INFO, () -> String.format("Server socket on socket %s closed", serverSocket)); + Level.INFO, + () -> + String.format("Server socket on socket %s closed", toString(serverSocketChannel))); } catch (IOException exception) { logger.log( Level.WARNING, exception, - () -> String.format("Closing server socket %s failed: %s", serverSocket, exception)); + () -> + String.format( + "Closing server socket %s failed: %s", + toString(serverSocketChannel), exception)); } } for (ConnectionHandler handler : this.handlers) { @@ -190,6 +218,14 @@ protected void doStop() { notifyStopped(); } + private String toString(ServerSocketChannel channel) { + try { + return channel.getLocalAddress().toString(); + } catch (IOException e) { + return "(unknown)"; + } + } + /** Safely stops the server (iff started), closing specific socket and cleaning up. */ public void stopServer() { stopAsync(); @@ -202,12 +238,12 @@ public void stopServer() { * @throws IOException if ServerSocket cannot start. */ void runTcpServer(CountDownLatch startupLatch, CountDownLatch stoppedLatch) throws IOException { - ServerSocket tcpSocket = - new ServerSocket(this.options.getProxyPort(), this.options.getMaxBacklog()); - this.serverSockets.add(tcpSocket); - this.localPort = tcpSocket.getLocalPort(); + ServerSocketChannel tcpSocketChannel = + this.serverSocketFactory.createTcpServerSocketChannel(this.options); + this.serverSocketChannels.add(tcpSocketChannel); + this.localPort = ((InetSocketAddress) tcpSocketChannel.getLocalAddress()).getPort(); tcpStartedLatch.countDown(); - runServer(tcpSocket, startupLatch, stoppedLatch); + runServer(tcpSocketChannel, startupLatch, stoppedLatch); } void runDomainSocketServer(CountDownLatch startupLatch, CountDownLatch stoppedLatch) @@ -218,38 +254,36 @@ void runDomainSocketServer(CountDownLatch startupLatch, CountDownLatch stoppedLa throw SpannerExceptionFactory.newSpannerException( ErrorCode.DEADLINE_EXCEEDED, "Timeout while waiting for TCP server to start"); } - File tempDir = new File(this.options.getSocketFile(getLocalPort())); + ServerSocketChannel channel = null; try { - if (tempDir.getParentFile() != null && !tempDir.getParentFile().exists()) { - tempDir.mkdirs(); - } - AFUNIXServerSocket domainSocket = AFUNIXServerSocket.newInstance(); - domainSocket.bind(AFUNIXSocketAddress.of(tempDir), this.options.getMaxBacklog()); - this.serverSockets.add(domainSocket); - runServer(domainSocket, startupLatch, stoppedLatch); + channel = serverSocketFactory.creatUnixDomainSocketChannel(options, getLocalPort()); + this.serverSocketChannels.add(channel); + runServer(channel, startupLatch, stoppedLatch); } catch (SocketException socketException) { logger.log( Level.SEVERE, String.format( "Failed to bind to Unix domain socket. Please verify that the user running PGAdapter has write permission for file %s", - tempDir), + channel == null ? "(unknown)" : channel.getLocalAddress()), socketException); startupLatch.countDown(); } } void runServer( - ServerSocket serverSocket, CountDownLatch startupLatch, CountDownLatch stoppedLatch) + ServerSocketChannel serverSocketChannel, + CountDownLatch startupLatch, + CountDownLatch stoppedLatch) throws IOException { startupLatch.countDown(); awaitRunning(); try { while (isRunning()) { - Socket socket = serverSocket.accept(); + SocketChannel socketChannel = serverSocketChannel.accept(); try { - createConnectionHandler(socket); + createConnectionHandler(socketChannel); } catch (SpannerException exception) { - handleConnectionError(exception, socket); + handleConnectionError(exception, socketChannel); } } } catch (SocketException e) { @@ -259,9 +293,9 @@ void runServer( () -> String.format( "Socket exception on socket %s: %s. This is normal when the server is stopped.", - serverSocket, e)); + serverSocketChannel, e)); } finally { - logger.log(Level.INFO, () -> String.format("Socket %s stopped", serverSocket)); + logger.log(Level.INFO, () -> String.format("Socket %s stopped", serverSocketChannel)); stoppedLatch.countDown(); } } @@ -270,9 +304,9 @@ void runServer( * Sends a message to the client that the connection could not be established. * * @param exception The exception that caused the connection request to fail. - * @param socket The socket that was created for the connection. + * @param socketChannel The socketChannel that was created for the connection. */ - private void handleConnectionError(SpannerException exception, Socket socket) { + private void handleConnectionError(SpannerException exception, SocketChannel socketChannel) { logger.log( Level.SEVERE, exception, @@ -282,7 +316,7 @@ private void handleConnectionError(SpannerException exception, Socket socket) { exception.getMessage())); try { DataOutputStream output = - new DataOutputStream(new BufferedOutputStream(socket.getOutputStream())); + new DataOutputStream(new BufferedOutputStream(Channels.newOutputStream(socketChannel))); new ErrorResponse(output, exception, ErrorResponse.State.ConnectionException, Severity.FATAL) .send(); output.flush(); @@ -297,12 +331,12 @@ private void handleConnectionError(SpannerException exception, Socket socket) { /** * Creates and runs the {@link ConnectionHandler}, saving an instance of it locally. * - * @param socket The socket the {@link ConnectionHandler} will read from. + * @param socketChannel The socketChannel the {@link ConnectionHandler} will read from. * @throws SpannerException if the {@link ConnectionHandler} is unable to connect to Cloud Spanner * or if the dialect of the database is not PostgreSQL. */ - void createConnectionHandler(Socket socket) { - ConnectionHandler handler = new ConnectionHandler(this, socket); + void createConnectionHandler(SocketChannel socketChannel) throws IOException { + ConnectionHandler handler = new ConnectionHandler(this, socketChannel); register(handler); handler.start(); } diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/ServerSocketFactory.java b/src/main/java/com/google/cloud/spanner/pgadapter/ServerSocketFactory.java new file mode 100644 index 000000000..500f9173b --- /dev/null +++ b/src/main/java/com/google/cloud/spanner/pgadapter/ServerSocketFactory.java @@ -0,0 +1,26 @@ +// Copyright 2022 Google LLC +// +// Licensed 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 com.google.cloud.spanner.pgadapter; + +import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata; +import java.io.IOException; +import java.nio.channels.ServerSocketChannel; + +interface ServerSocketFactory { + ServerSocketChannel createTcpServerSocketChannel(OptionsMetadata options) throws IOException; + + ServerSocketChannel creatUnixDomainSocketChannel(OptionsMetadata options, int localPort) + throws IOException; +} diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/ConnectionHandlerTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/ConnectionHandlerTest.java index 0e5e65e09..eb8c67240 100644 --- a/src/test/java/com/google/cloud/spanner/pgadapter/ConnectionHandlerTest.java +++ b/src/test/java/com/google/cloud/spanner/pgadapter/ConnectionHandlerTest.java @@ -17,11 +17,9 @@ import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; import java.io.IOException; -import java.net.InetAddress; -import java.net.Socket; +import java.nio.channels.ByteChannel; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -32,40 +30,33 @@ public class ConnectionHandlerTest { @Test public void testTerminateClosesSocket() throws IOException { ProxyServer server = mock(ProxyServer.class); - Socket socket = mock(Socket.class); - InetAddress address = mock(InetAddress.class); - when(socket.getInetAddress()).thenReturn(address); + ByteChannel channel = mock(ByteChannel.class); - ConnectionHandler connection = new ConnectionHandler(server, socket); + ConnectionHandler connection = new ConnectionHandler(server, channel); connection.terminate(); - verify(socket).close(); + verify(channel).close(); } @Test public void testTerminateDoesNotCloseSocketTwice() throws IOException { ProxyServer server = mock(ProxyServer.class); - Socket socket = mock(Socket.class); - when(socket.isClosed()).thenReturn(false, true); - InetAddress address = mock(InetAddress.class); - when(socket.getInetAddress()).thenReturn(address); + ByteChannel channel = mock(ByteChannel.class); - ConnectionHandler connection = new ConnectionHandler(server, socket); + ConnectionHandler connection = new ConnectionHandler(server, channel); connection.terminate(); // Calling terminate a second time should be a no-op. connection.terminate(); // Verify that close was only called once. - verify(socket).close(); + verify(channel).close(); } @Test public void testTerminateHandlesCloseError() throws IOException { ProxyServer server = mock(ProxyServer.class); - Socket socket = mock(Socket.class); - InetAddress address = mock(InetAddress.class); - when(socket.getInetAddress()).thenReturn(address); + ByteChannel socket = mock(ByteChannel.class); // IOException should be handled internally in terminate(). doThrow(new IOException("test exception")).when(socket).close(); From 16eeecc4db74af0736cdb6295603f377d45812bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Tue, 14 Jun 2022 18:05:38 +0200 Subject: [PATCH 02/25] fix: delete socket file on exit --- .../spanner/pgadapter/Java17ServerSocketFactory.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/Java17ServerSocketFactory.java b/src/main/java/com/google/cloud/spanner/pgadapter/Java17ServerSocketFactory.java index 38a6c0a7f..530cca2dd 100644 --- a/src/main/java/com/google/cloud/spanner/pgadapter/Java17ServerSocketFactory.java +++ b/src/main/java/com/google/cloud/spanner/pgadapter/Java17ServerSocketFactory.java @@ -50,11 +50,12 @@ public ServerSocketChannel createTcpServerSocketChannel(OptionsMetadata options) public ServerSocketChannel creatUnixDomainSocketChannel(OptionsMetadata options, int localPort) throws IOException { - File tempDir = new File(options.getSocketFile(localPort)); - Path path = Path.of(tempDir.toURI()); - if (tempDir.getParentFile() != null && !tempDir.getParentFile().exists()) { - tempDir.mkdirs(); + File socketFile = new File(options.getSocketFile(localPort)); + Path path = Path.of(socketFile.toURI()); + if (socketFile.getParentFile() != null && !socketFile.getParentFile().exists()) { + socketFile.mkdirs(); } + socketFile.deleteOnExit(); UnixDomainSocketAddress address = UnixDomainSocketAddress.of(path); ServerSocketChannel domainSocketChannel = ServerSocketChannel.open(StandardProtocolFamily.UNIX); domainSocketChannel.configureBlocking(true); From 716cc11c28587c3881f0c7158124121e23e6a858 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 15 Jun 2022 13:15:06 +0200 Subject: [PATCH 03/25] test: run integration tests against native image --- .github/workflows/native-image.yaml | 56 +++++++++++++++++++ .../pgadapter/Java17ServerSocketFactory.java | 12 +++- .../pgadapter/Java8ServerSocketFactory.java | 17 ++++-- .../cloud/spanner/pgadapter/ITAuthTest.java | 3 + .../ITJdbcDescribeStatementTest.java | 4 +- .../spanner/pgadapter/ITJdbcMetadataTest.java | 4 +- .../cloud/spanner/pgadapter/ITJdbcTest.java | 8 +++ .../cloud/spanner/pgadapter/ITQueryTest.java | 5 ++ .../spanner/pgadapter/PgAdapterTestEnv.java | 31 +++++++++- .../spanner/pgadapter/golang/ITPgxTest.java | 13 +++-- .../spanner/pgadapter/golang/PgxTest.java | 3 +- 11 files changed, 143 insertions(+), 13 deletions(-) create mode 100644 .github/workflows/native-image.yaml diff --git a/.github/workflows/native-image.yaml b/.github/workflows/native-image.yaml new file mode 100644 index 000000000..8c5dd8379 --- /dev/null +++ b/.github/workflows/native-image.yaml @@ -0,0 +1,56 @@ +on: + # This allows manual activation of this action for testing. + workflow_dispatch: + pull_request: +name: native-image +env: + GOOGLE_CLOUD_PROJECT: "span-cloud-testing" + GOOGLE_CLOUD_INSTANCE: "pgadapter-testing" + GOOGLE_CLOUD_DATABASE: "testdb_integration" + GOOGLE_CLOUD_ENDPOINT: "spanner.googleapis.com" +jobs: + check-env: + outputs: + has-key: ${{ steps.project-id.outputs.defined }} + runs-on: ubuntu-latest + steps: + - id: project-id + env: + GCP_PROJECT_ID: ${{ secrets.GCP_PROJECT_ID }} + if: "${{ env.GCP_PROJECT_ID != '' }}" + run: echo "::set-output name=defined::true" + + build-native-image: + needs: [check-env] + if: needs.check-env.outputs.has-key == 'true' + timeout-minutes: 60 + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: graalvm/setup-graalvm@v1 + with: + version: 'latest' + java-version: '17' + components: 'native-image' + github-token: ${{ secrets.GITHUB_TOKEN }} + - name: Setup GCloud + uses: google-github-actions/setup-gcloud@v0 + with: + project_id: ${{ secrets.GCP_PROJECT_ID }} + service_account_key: ${{ secrets.JSON_SERVICE_ACCOUNT_CREDENTIALS }} + export_default_credentials: true + - name: Build and run PGAdapter native image + run: | + mvn package -Passembly -Pnative-image -DskipTests + cd "target/pgadapter" + native-image -H:IncludeResources=".*metadata.*json$" -jar pgadapter.jar --no-fallback + ./pgadapter -p ${{env.GOOGLE_CLOUD_PROJECT}} -i ${{env.GOOGLE_CLOUD_INSTANCE}} & + - name: Run integration tests + run: | + mvn verify \ + -Dclirr.skip=true \ + -DskipITs=false \ + -DPG_ADAPTER_ADDRESS="localhost" \ + -DPG_ADAPTER_SOCKET_DIR="/tmp" \ + -DPG_ADAPTER_LOCAL_PORT="5432" \ + -DPG_ADAPTER_DATABASE="${{env.GOOGLE_CLOUD_DATABASE}}" diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/Java17ServerSocketFactory.java b/src/main/java/com/google/cloud/spanner/pgadapter/Java17ServerSocketFactory.java index 530cca2dd..811476199 100644 --- a/src/main/java/com/google/cloud/spanner/pgadapter/Java17ServerSocketFactory.java +++ b/src/main/java/com/google/cloud/spanner/pgadapter/Java17ServerSocketFactory.java @@ -21,6 +21,7 @@ import java.net.StandardProtocolFamily; import java.net.UnixDomainSocketAddress; import java.nio.channels.ServerSocketChannel; +import java.nio.file.Files; import java.nio.file.Path; // If your IDE is complaining about this file not being compatible with the language level @@ -55,7 +56,16 @@ public ServerSocketChannel creatUnixDomainSocketChannel(OptionsMetadata options, if (socketFile.getParentFile() != null && !socketFile.getParentFile().exists()) { socketFile.mkdirs(); } - socketFile.deleteOnExit(); + + // deleteOnExit() does not work when PGAdapter is built as a native image. It also does not get + // deleted if the JVM crashes. We therefore need to try to delete the file at startup to ensure + // we don't get a lot of 'address already in use' errors. + try { + Files.deleteIfExists(path); + } catch (IOException ignore) { + // Ignore and let the Unix domain socket subsystem throw an 'Address in use' error. + } + UnixDomainSocketAddress address = UnixDomainSocketAddress.of(path); ServerSocketChannel domainSocketChannel = ServerSocketChannel.open(StandardProtocolFamily.UNIX); domainSocketChannel.configureBlocking(true); diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/Java8ServerSocketFactory.java b/src/main/java/com/google/cloud/spanner/pgadapter/Java8ServerSocketFactory.java index 4f2a3670a..3fd27c567 100644 --- a/src/main/java/com/google/cloud/spanner/pgadapter/Java8ServerSocketFactory.java +++ b/src/main/java/com/google/cloud/spanner/pgadapter/Java8ServerSocketFactory.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.net.InetSocketAddress; import java.nio.channels.ServerSocketChannel; +import java.nio.file.Files; import org.newsclub.net.unix.AFUNIXServerSocketChannel; import org.newsclub.net.unix.AFUNIXSocketAddress; @@ -38,12 +39,20 @@ public ServerSocketChannel createTcpServerSocketChannel(OptionsMetadata options) public ServerSocketChannel creatUnixDomainSocketChannel(OptionsMetadata options, int localPort) throws IOException { - File tempDir = new File(options.getSocketFile(localPort)); - if (tempDir.getParentFile() != null && !tempDir.getParentFile().exists()) { - tempDir.mkdirs(); + File socketFile = new File(options.getSocketFile(localPort)); + if (socketFile.getParentFile() != null && !socketFile.getParentFile().exists()) { + socketFile.mkdirs(); + } + // deleteOnExit() does not work when PGAdapter is built as a native image. It also does not get + // deleted if the JVM crashes. We therefore need to try to delete the file at startup to ensure + // we don't get a lot of 'address already in use' errors. + try { + Files.deleteIfExists(socketFile.toPath()); + } catch (IOException ignore) { + // Ignore and let the Unix domain socket subsystem throw an 'Address in use' error. } AFUNIXServerSocketChannel channel = AFUNIXServerSocketChannel.open(); - channel.bind(AFUNIXSocketAddress.of(tempDir), options.getMaxBacklog()); + channel.bind(AFUNIXSocketAddress.of(socketFile), options.getMaxBacklog()); return channel; } diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/ITAuthTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/ITAuthTest.java index e1c72c779..bdfa2984c 100644 --- a/src/test/java/com/google/cloud/spanner/pgadapter/ITAuthTest.java +++ b/src/test/java/com/google/cloud/spanner/pgadapter/ITAuthTest.java @@ -18,6 +18,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; import com.google.api.client.json.GenericJson; @@ -51,6 +52,8 @@ public String getCredentials() { @BeforeClass public static void setup() throws ClassNotFoundException { + assumeFalse( + "Auth test requires PGAdapter to be started with -a", testEnv.isUsingExternalPGAdapter()); // Make sure the PG JDBC driver is loaded. Class.forName("org.postgresql.Driver"); diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcDescribeStatementTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcDescribeStatementTest.java index 34e63faca..20400c11b 100644 --- a/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcDescribeStatementTest.java +++ b/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcDescribeStatementTest.java @@ -101,7 +101,9 @@ public void insertTestData() { } private String getConnectionUrl() { - return String.format("jdbc:postgresql://%s/", testEnv.getPGAdapterHostAndPort()); + return String.format( + "jdbc:postgresql://%s/%s", + testEnv.getPGAdapterHostAndPort(), database.getId().getDatabase()); } @Test diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcMetadataTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcMetadataTest.java index 182fecfd8..6cd84cfb9 100644 --- a/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcMetadataTest.java +++ b/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcMetadataTest.java @@ -106,7 +106,9 @@ private static Iterable getDdlStatements() { } private String createUrl() { - return String.format("jdbc:postgresql://%s/", testEnv.getPGAdapterHostAndPort()); + return String.format( + "jdbc:postgresql://%s/%s", + testEnv.getPGAdapterHostAndPort(), database.getId().getDatabase()); } private void runForAllVersions(Consumer runnable) throws Exception { diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcTest.java index 9c3b6ce39..2157d1692 100644 --- a/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcTest.java +++ b/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcTest.java @@ -134,6 +134,14 @@ public void insertTestData() { } private String getConnectionUrl() { + if (useDomainSocket) { + return String.format( + "jdbc:postgresql://localhost/%s?" + + "socketFactory=org.newsclub.net.unix.AFUNIXSocketFactory$FactoryArg" + + "&socketFactoryArg=%s" + + "&preferQueryMode=%s", + database.getId().getDatabase(), testEnv.getPGAdapterSocketFile(), preferQueryMode); + } return String.format( "jdbc:postgresql://%s/%s?preferQueryMode=%s", testEnv.getPGAdapterHostAndPort(), database.getId().getDatabase(), preferQueryMode); diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/ITQueryTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/ITQueryTest.java index cdf70613a..7a0d2a79c 100644 --- a/src/test/java/com/google/cloud/spanner/pgadapter/ITQueryTest.java +++ b/src/test/java/com/google/cloud/spanner/pgadapter/ITQueryTest.java @@ -16,6 +16,7 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assume.assumeFalse; import com.google.cloud.spanner.Database; import com.google.common.collect.ImmutableList; @@ -44,6 +45,10 @@ public final class ITQueryTest implements IntegrationTest { @BeforeClass public static void setup() { + assumeFalse( + "Query test does not support connecting to a specific database", + testEnv.isUsingExternalPGAdapter()); + testEnv.setUp(); database = testEnv.createDatabase(getDdlStatements()); testEnv.startPGAdapterServerWithDefaultDatabase(database.getId(), Collections.emptyList()); diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/PgAdapterTestEnv.java b/src/test/java/com/google/cloud/spanner/pgadapter/PgAdapterTestEnv.java index 034c4937e..d6f2d2631 100644 --- a/src/test/java/com/google/cloud/spanner/pgadapter/PgAdapterTestEnv.java +++ b/src/test/java/com/google/cloud/spanner/pgadapter/PgAdapterTestEnv.java @@ -36,6 +36,7 @@ import com.google.spanner.admin.database.v1.UpdateDatabaseDdlMetadata; import java.io.DataInputStream; import java.io.DataOutputStream; +import java.io.File; import java.io.FileInputStream; import java.io.IOException; import java.nio.ByteBuffer; @@ -76,8 +77,12 @@ public class PgAdapterTestEnv { // PgAdapter host address (when using an external PGAdapter instance). public static final String PG_ADAPTER_ADDRESS = System.getProperty("PG_ADAPTER_ADDRESS", null); + // PgAdapter socket directory (when using an external PGAdapter instance). + public static final String PG_ADAPTER_SOCKET_DIR = + System.getProperty("PG_ADAPTER_SOCKET_DIR", "/tmp"); + // PgAdapter port should be set through this system property. - public static final String PG_ADAPTER_PORT = "PG_ADAPTER_PORT"; + public static final String PG_ADAPTER_PORT = System.getProperty("PG_ADAPTER_PORT", "5432"); // Environment variable that can be used to force the test env to assume that the test database // already exists. This can be used to speed up local testing by manually creating the test @@ -217,6 +222,15 @@ public String getCredentials() { return gcpCredentials; } + /** + * Returns true if the current test environment is using an externally started instance of + * PGAdapter. Some tests do not support this, as they require PGAdapter to be started with + * specific arguments. + */ + public boolean isUsingExternalPGAdapter() { + return PG_ADAPTER_ADDRESS != null; + } + public String getPGAdapterHostAndPort() { if (server != null) { return String.format("localhost:%d", server.getLocalPort()); @@ -231,6 +245,21 @@ public String getPGAdapterHost() { return PG_ADAPTER_ADDRESS; } + public String getPGAdapterSocketDir() { + if (server != null) { + String file = server.getOptions().getSocketFile(getPGAdapterPort()); + return new File(file).toPath().getParent().toString(); + } + return PG_ADAPTER_SOCKET_DIR; + } + + public String getPGAdapterSocketFile() { + if (server != null) { + return server.getOptions().getSocketFile(getPGAdapterPort()); + } + return String.format("%s/.s.PGSQL.%d", PG_ADAPTER_SOCKET_DIR, getPGAdapterPort()); + } + public int getPGAdapterPort() { if (server != null) { return server.getLocalPort(); diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/golang/ITPgxTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/golang/ITPgxTest.java index 7be53f864..fb86c14e1 100644 --- a/src/test/java/com/google/cloud/spanner/pgadapter/golang/ITPgxTest.java +++ b/src/test/java/com/google/cloud/spanner/pgadapter/golang/ITPgxTest.java @@ -110,13 +110,18 @@ private GoString createConnString() { if (useDomainSocket) { return new GoString( String.format( - "host=/tmp port=%d prefer_simple_protocol=%s", - testEnv.getServer().getLocalPort(), preferQueryMode.equals("simple"))); + "host=%s port=%d database=%s prefer_simple_protocol=%s", + testEnv.getPGAdapterSocketDir(), + testEnv.getPGAdapterPort(), + database.getId().getDatabase(), + preferQueryMode.equals("simple"))); } return new GoString( String.format( - "postgres://uid:pwd@localhost:%d/?sslmode=disable&prefer_simple_protocol=%s", - testEnv.getServer().getLocalPort(), preferQueryMode.equals("simple"))); + "postgres://uid:pwd@%s/%s?sslmode=disable&prefer_simple_protocol=%s", + testEnv.getPGAdapterHostAndPort(), + database.getId().getDatabase(), + preferQueryMode.equals("simple"))); } @Before diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/golang/PgxTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/golang/PgxTest.java index cfb8345d2..7f333d81c 100644 --- a/src/test/java/com/google/cloud/spanner/pgadapter/golang/PgxTest.java +++ b/src/test/java/com/google/cloud/spanner/pgadapter/golang/PgxTest.java @@ -48,7 +48,8 @@ public interface PgxTest extends Library { static PgxTest compile() throws IOException, InterruptedException { // Compile the Go code to ensure that we always have the most recent test code. ProcessBuilder builder = new ProcessBuilder(); - String[] compileCommand = "go build -o pgx_test.so -buildmode=c-shared pgx.go".split(" "); + String[] compileCommand = + "/usr/local/go/bin/go build -o pgx_test.so -buildmode=c-shared pgx.go".split(" "); builder.command(compileCommand); builder.directory(new File("./src/test/golang/pgadapter_pgx_tests")); Process process = builder.start(); From 63e6c0c5ba8e46d993b97706130214c6443fd181 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 15 Jun 2022 13:52:35 +0200 Subject: [PATCH 04/25] test: fix Go tests + increase heap memory for native image --- .github/workflows/native-image.yaml | 5 +++-- .../com/google/cloud/spanner/pgadapter/golang/PgxTest.java | 3 +-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/native-image.yaml b/.github/workflows/native-image.yaml index 8c5dd8379..20760cdac 100644 --- a/.github/workflows/native-image.yaml +++ b/.github/workflows/native-image.yaml @@ -1,7 +1,8 @@ on: # This allows manual activation of this action for testing. workflow_dispatch: - pull_request: + schedule: + - cron: '0 2 * * 1,2,3,4,5' name: native-image env: GOOGLE_CLOUD_PROJECT: "span-cloud-testing" @@ -43,7 +44,7 @@ jobs: run: | mvn package -Passembly -Pnative-image -DskipTests cd "target/pgadapter" - native-image -H:IncludeResources=".*metadata.*json$" -jar pgadapter.jar --no-fallback + native-image -J-Xmx12g -H:IncludeResources=".*metadata.*json$" -jar pgadapter.jar --no-fallback ./pgadapter -p ${{env.GOOGLE_CLOUD_PROJECT}} -i ${{env.GOOGLE_CLOUD_INSTANCE}} & - name: Run integration tests run: | diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/golang/PgxTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/golang/PgxTest.java index 7f333d81c..cfb8345d2 100644 --- a/src/test/java/com/google/cloud/spanner/pgadapter/golang/PgxTest.java +++ b/src/test/java/com/google/cloud/spanner/pgadapter/golang/PgxTest.java @@ -48,8 +48,7 @@ public interface PgxTest extends Library { static PgxTest compile() throws IOException, InterruptedException { // Compile the Go code to ensure that we always have the most recent test code. ProcessBuilder builder = new ProcessBuilder(); - String[] compileCommand = - "/usr/local/go/bin/go build -o pgx_test.so -buildmode=c-shared pgx.go".split(" "); + String[] compileCommand = "go build -o pgx_test.so -buildmode=c-shared pgx.go".split(" "); builder.command(compileCommand); builder.directory(new File("./src/test/golang/pgadapter_pgx_tests")); Process process = builder.start(); From d3cb9c83e3b05d52838355042de269bab0d6c972 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 15 Jun 2022 13:55:22 +0200 Subject: [PATCH 05/25] build: build native image for pull requests --- .github/workflows/native-image.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/native-image.yaml b/.github/workflows/native-image.yaml index 20760cdac..fd30d15a3 100644 --- a/.github/workflows/native-image.yaml +++ b/.github/workflows/native-image.yaml @@ -1,6 +1,7 @@ on: # This allows manual activation of this action for testing. workflow_dispatch: + pull_request: schedule: - cron: '0 2 * * 1,2,3,4,5' name: native-image From 05028350e54e7e6714eecb47b52251c11d23ea37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 15 Jun 2022 14:47:16 +0200 Subject: [PATCH 06/25] build: build native image on mac --- .github/workflows/native-image.yaml | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/.github/workflows/native-image.yaml b/.github/workflows/native-image.yaml index fd30d15a3..2700dc884 100644 --- a/.github/workflows/native-image.yaml +++ b/.github/workflows/native-image.yaml @@ -26,7 +26,7 @@ jobs: needs: [check-env] if: needs.check-env.outputs.has-key == 'true' timeout-minutes: 60 - runs-on: ubuntu-latest + runs-on: macos-latest steps: - uses: actions/checkout@v2 - uses: graalvm/setup-graalvm@v1 @@ -41,11 +41,16 @@ jobs: project_id: ${{ secrets.GCP_PROJECT_ID }} service_account_key: ${{ secrets.JSON_SERVICE_ACCOUNT_CREDENTIALS }} export_default_credentials: true + - name: Set up swap space + if: runner.os == 'Linux' + uses: pierotofy/set-swap-space@v1.0 + with: + swap-size-gb: 12 - name: Build and run PGAdapter native image run: | mvn package -Passembly -Pnative-image -DskipTests cd "target/pgadapter" - native-image -J-Xmx12g -H:IncludeResources=".*metadata.*json$" -jar pgadapter.jar --no-fallback + native-image -J-Xmx14g -H:IncludeResources=".*metadata.*json$" -jar pgadapter.jar --no-fallback ./pgadapter -p ${{env.GOOGLE_CLOUD_PROJECT}} -i ${{env.GOOGLE_CLOUD_INSTANCE}} & - name: Run integration tests run: | @@ -54,5 +59,5 @@ jobs: -DskipITs=false \ -DPG_ADAPTER_ADDRESS="localhost" \ -DPG_ADAPTER_SOCKET_DIR="/tmp" \ - -DPG_ADAPTER_LOCAL_PORT="5432" \ + -DPG_ADAPTER_PORT="5432" \ -DPG_ADAPTER_DATABASE="${{env.GOOGLE_CLOUD_DATABASE}}" From c3c8cf1d4f03a6b584c9aba4ad104c4b316d783e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 15 Jun 2022 15:30:53 +0200 Subject: [PATCH 07/25] fix: use same instance for both PGAdapter and the tests --- .github/workflows/native-image.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/native-image.yaml b/.github/workflows/native-image.yaml index 2700dc884..04c7dc6b6 100644 --- a/.github/workflows/native-image.yaml +++ b/.github/workflows/native-image.yaml @@ -57,7 +57,9 @@ jobs: mvn verify \ -Dclirr.skip=true \ -DskipITs=false \ + -DPG_ADAPTER_PROJECT="${{env.GOOGLE_CLOUD_PROJECT}}" + -DPG_ADAPTER_INSTANCE="${{env.GOOGLE_CLOUD_INSTANCE}}" + -DPG_ADAPTER_DATABASE="${{env.GOOGLE_CLOUD_DATABASE}}" -DPG_ADAPTER_ADDRESS="localhost" \ -DPG_ADAPTER_SOCKET_DIR="/tmp" \ -DPG_ADAPTER_PORT="5432" \ - -DPG_ADAPTER_DATABASE="${{env.GOOGLE_CLOUD_DATABASE}}" From d07aaa28fe64b94386332157c8aaf92839c00d41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 15 Jun 2022 16:21:16 +0200 Subject: [PATCH 08/25] fix: missing backslashes at the end of lines --- .github/workflows/native-image.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/native-image.yaml b/.github/workflows/native-image.yaml index 04c7dc6b6..ffdb1b0e5 100644 --- a/.github/workflows/native-image.yaml +++ b/.github/workflows/native-image.yaml @@ -57,9 +57,9 @@ jobs: mvn verify \ -Dclirr.skip=true \ -DskipITs=false \ - -DPG_ADAPTER_PROJECT="${{env.GOOGLE_CLOUD_PROJECT}}" - -DPG_ADAPTER_INSTANCE="${{env.GOOGLE_CLOUD_INSTANCE}}" - -DPG_ADAPTER_DATABASE="${{env.GOOGLE_CLOUD_DATABASE}}" + -DPG_ADAPTER_PROJECT="${{env.GOOGLE_CLOUD_PROJECT}}" \ + -DPG_ADAPTER_INSTANCE="${{env.GOOGLE_CLOUD_INSTANCE}}" \ + -DPG_ADAPTER_DATABASE="${{env.GOOGLE_CLOUD_DATABASE}}" \ -DPG_ADAPTER_ADDRESS="localhost" \ -DPG_ADAPTER_SOCKET_DIR="/tmp" \ - -DPG_ADAPTER_PORT="5432" \ + -DPG_ADAPTER_PORT="5432" From acb8988b15661bd822f5495812839ea948f8072f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Thu, 16 Jun 2022 10:21:07 +0200 Subject: [PATCH 09/25] fix: buffer size must be at least 65k on MacOS --- pom.xml | 4 +- .../spanner/pgadapter/ConnectionHandler.java | 5 +- .../cloud/spanner/pgadapter/ProxyServer.java | 3 +- .../pgadapter/CopyInMockServerTest.java | 144 +++++++++++++++++- 4 files changed, 149 insertions(+), 7 deletions(-) diff --git a/pom.xml b/pom.xml index 829b5444a..f3243a154 100644 --- a/pom.xml +++ b/pom.xml @@ -79,13 +79,13 @@ com.kohlschutter.junixsocket junixsocket-core - 2.4.0 + 2.5.0 pom com.kohlschutter.junixsocket junixsocket-common - 2.4.0 + 2.5.0 com.googlecode.json-simple diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/ConnectionHandler.java b/src/main/java/com/google/cloud/spanner/pgadapter/ConnectionHandler.java index 0497964ac..12496d37d 100644 --- a/src/main/java/com/google/cloud/spanner/pgadapter/ConnectionHandler.java +++ b/src/main/java/com/google/cloud/spanner/pgadapter/ConnectionHandler.java @@ -201,10 +201,11 @@ public void run() { "Connection handler with ID %s starting for client %s", getName(), remoteClient)); try (DataInputStream input = - new DataInputStream(new BufferedInputStream(Channels.newInputStream(socketChannel))); + new DataInputStream( + new BufferedInputStream(Channels.newInputStream(socketChannel), 1 << 16)); DataOutputStream output = new DataOutputStream( - new BufferedOutputStream(Channels.newOutputStream(socketChannel)))) { + new BufferedOutputStream(Channels.newOutputStream(socketChannel), 1 << 16))) { if (!server.getOptions().disableLocalhostCheck() && this.remoteAddress != null && !this.remoteAddress.getAddress().isAnyLocalAddress() diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/ProxyServer.java b/src/main/java/com/google/cloud/spanner/pgadapter/ProxyServer.java index 45a956251..bc331ee11 100644 --- a/src/main/java/com/google/cloud/spanner/pgadapter/ProxyServer.java +++ b/src/main/java/com/google/cloud/spanner/pgadapter/ProxyServer.java @@ -316,7 +316,8 @@ private void handleConnectionError(SpannerException exception, SocketChannel soc exception.getMessage())); try { DataOutputStream output = - new DataOutputStream(new BufferedOutputStream(Channels.newOutputStream(socketChannel))); + new DataOutputStream( + new BufferedOutputStream(Channels.newOutputStream(socketChannel), 1 << 16)); new ErrorResponse(output, exception, ErrorResponse.State.ConnectionException, Severity.FATAL) .send(); output.flush(); diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/CopyInMockServerTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/CopyInMockServerTest.java index 38dea69a6..648a3ba77 100644 --- a/src/test/java/com/google/cloud/spanner/pgadapter/CopyInMockServerTest.java +++ b/src/test/java/com/google/cloud/spanner/pgadapter/CopyInMockServerTest.java @@ -21,6 +21,7 @@ import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime; import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult; +import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata; import com.google.protobuf.ListValue; import com.google.protobuf.Value; import com.google.spanner.v1.CommitRequest; @@ -33,6 +34,7 @@ import com.google.spanner.v1.TypeCode; import io.grpc.Status; import java.io.File; +import java.io.FileInputStream; import java.io.IOException; import java.io.StringReader; import java.nio.charset.StandardCharsets; @@ -44,7 +46,10 @@ import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; +import org.postgresql.PGConnection; import org.postgresql.copy.CopyIn; import org.postgresql.copy.CopyManager; import org.postgresql.core.BaseConnection; @@ -53,9 +58,17 @@ import org.postgresql.core.v3.CopyOperationImpl; import org.postgresql.core.v3.QueryExecutorImpl; -@RunWith(JUnit4.class) +@RunWith(Parameterized.class) public class CopyInMockServerTest extends AbstractMockServerTest { + @Parameter public boolean useDomainSocket; + + @Parameters(name = "useDomainSocket = {0}") + public static Object[] data() { + OptionsMetadata options = new OptionsMetadata(new String[] {"-p p", "-i i"}); + return options.isDomainSocketEnabled() ? new Object[] {true, false} : new Object[] {false}; + } + @BeforeClass public static void loadPgJdbcDriver() throws Exception { // Make sure the PG JDBC driver is loaded. @@ -67,6 +80,13 @@ public static void loadPgJdbcDriver() throws Exception { * mode for queries and DML statements. */ private String createUrl() { + if (useDomainSocket) { + return String.format( + "jdbc:postgresql://localhost/?" + + "socketFactory=org.newsclub.net.unix.AFUNIXSocketFactory$FactoryArg" + + "&socketFactoryArg=/tmp/.s.PGSQL.%d", + pgServer.getLocalPort()); + } return String.format("jdbc:postgresql://localhost:%d/", pgServer.getLocalPort()); } @@ -96,6 +116,21 @@ public void testCopyIn() throws SQLException, IOException { assertEquals(3, mutation.getInsert().getValuesCount()); } + @Test + public void testCopyIn_Small() throws SQLException, IOException { + setupCopyInformationSchemaResults(); + + try (Connection connection = DriverManager.getConnection(createUrl())) { + PGConnection pgConnection = connection.unwrap(PGConnection.class); + CopyManager copyManager = pgConnection.getCopyAPI(); + long copyCount = + copyManager.copyIn( + "copy all_types from stdin;", + new FileInputStream("./src/test/resources/all_types_data_small.txt")); + assertEquals(100L, copyCount); + } + } + @Test public void testCopyInError() throws SQLException { setupCopyInformationSchemaResults(); @@ -365,6 +400,64 @@ private void setupCopyInformationSchemaResults(boolean tableFound) { .to("users") .build(), resultSet)); + com.google.spanner.v1.ResultSet allTypesResultSet = + com.google.spanner.v1.ResultSet.newBuilder() + .addRows( + ListValue.newBuilder() + .addValues(Value.newBuilder().setStringValue("col_bigint").build()) + .addValues(Value.newBuilder().setStringValue("bigint").build()) + .build()) + .addRows( + ListValue.newBuilder() + .addValues(Value.newBuilder().setStringValue("col_bool").build()) + .addValues(Value.newBuilder().setStringValue("boolean").build()) + .build()) + .addRows( + ListValue.newBuilder() + .addValues(Value.newBuilder().setStringValue("col_bytea").build()) + .addValues(Value.newBuilder().setStringValue("bytea").build()) + .build()) + .addRows( + ListValue.newBuilder() + .addValues(Value.newBuilder().setStringValue("col_float8").build()) + .addValues(Value.newBuilder().setStringValue("float8").build()) + .build()) + .addRows( + ListValue.newBuilder() + .addValues(Value.newBuilder().setStringValue("col_int").build()) + .addValues(Value.newBuilder().setStringValue("bigint").build()) + .build()) + .addRows( + ListValue.newBuilder() + .addValues(Value.newBuilder().setStringValue("col_numeric").build()) + .addValues(Value.newBuilder().setStringValue("numeric").build()) + .build()) + .addRows( + ListValue.newBuilder() + .addValues(Value.newBuilder().setStringValue("col_timestamptz").build()) + .addValues( + Value.newBuilder().setStringValue("timestamp with time zone").build()) + .build()) + .addRows( + ListValue.newBuilder() + .addValues(Value.newBuilder().setStringValue("col_date").build()) + .addValues(Value.newBuilder().setStringValue("date").build()) + .build()) + .addRows( + ListValue.newBuilder() + .addValues(Value.newBuilder().setStringValue("col_varchar").build()) + .addValues(Value.newBuilder().setStringValue("character varying").build()) + .build()) + .setMetadata(metadata) + .build(); + mockSpanner.putStatementResult( + StatementResult.query( + com.google.cloud.spanner.Statement.newBuilder( + "SELECT column_name, data_type FROM information_schema.columns WHERE table_name = $1") + .bind("p1") + .to("all_types") + .build(), + allTypesResultSet)); String indexedColumnsCountSql = "SELECT COUNT(*) FROM information_schema.index_columns WHERE table_schema='public' and table_name=$1 and column_name in ($2, $3, $4)"; @@ -400,5 +493,52 @@ private void setupCopyInformationSchemaResults(boolean tableFound) { .to("name") .build(), indexedColumnsCountResultSet)); + + String allTypesIndexedColumnsCountSql = + "SELECT COUNT(*) FROM information_schema.index_columns WHERE table_schema='public' and table_name=$1 and column_name in ($2, $3, $4, $5, $6, $7, $8, $9, $10)"; + ResultSetMetadata allTypesIndexedColumnsCountMetadata = + ResultSetMetadata.newBuilder() + .setRowType( + StructType.newBuilder() + .addFields( + Field.newBuilder() + .setName("") + .setType(Type.newBuilder().setCode(TypeCode.INT64).build()) + .build()) + .build()) + .build(); + com.google.spanner.v1.ResultSet allTypesIndexedColumnsCountResultSet = + com.google.spanner.v1.ResultSet.newBuilder() + .addRows( + ListValue.newBuilder() + .addValues(Value.newBuilder().setStringValue("1").build()) + .build()) + .setMetadata(allTypesIndexedColumnsCountMetadata) + .build(); + mockSpanner.putStatementResult( + StatementResult.query( + com.google.cloud.spanner.Statement.newBuilder(allTypesIndexedColumnsCountSql) + .bind("p1") + .to("all_types") + .bind("p2") + .to("col_bigint") + .bind("p3") + .to("col_bool") + .bind("p4") + .to("col_bytea") + .bind("p5") + .to("col_float8") + .bind("p6") + .to("col_int") + .bind("p7") + .to("col_numeric") + .bind("p8") + .to("col_timestamptz") + .bind("p9") + .to("col_date") + .bind("p10") + .to("col_varchar") + .build(), + allTypesIndexedColumnsCountResultSet)); } } From 8639a9e60bb097721bc011b52911fcf066881510 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Fri, 17 Jun 2022 14:52:51 +0200 Subject: [PATCH 10/25] build: also run native image unit tests --- .github/workflows/integration.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/integration.yaml b/.github/workflows/integration.yaml index 180f34776..3e99bc603 100644 --- a/.github/workflows/integration.yaml +++ b/.github/workflows/integration.yaml @@ -47,6 +47,8 @@ jobs: - run: pip install -r ./src/test/python/requirements.txt - name: Run unit tests run: mvn test -B -Ptest-all + - name: Run unit tests for native image + run: mvn test -B -Pnative-image - name: Setup GCloud uses: google-github-actions/setup-gcloud@v0 with: From 338bf7cc550c940ea8bbd9a446c8ac2f45f99859 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Fri, 17 Jun 2022 15:01:04 +0200 Subject: [PATCH 11/25] build: use Java 17 for integration tests to enable native image testing --- .github/workflows/integration.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/integration.yaml b/.github/workflows/integration.yaml index 48ac55520..068dcd1a5 100644 --- a/.github/workflows/integration.yaml +++ b/.github/workflows/integration.yaml @@ -29,7 +29,7 @@ jobs: uses: actions/setup-java@v3 with: distribution: zulu - java-version: 8 + java-version: 17 - run: java -version - name: Setup Go uses: actions/setup-go@v3 From 1d1dec88a95883fa90baa24c3b4cdcce09ea123a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Sat, 18 Jun 2022 15:23:44 +0200 Subject: [PATCH 12/25] fix: add release version + parameterize junixsockets version --- pom.xml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index d5edeb099..3c780f879 100644 --- a/pom.xml +++ b/pom.xml @@ -284,14 +284,14 @@ com.kohlschutter.junixsocket junixsocket-core - 2.4.0 + ${junixsocket.version} pom test com.kohlschutter.junixsocket junixsocket-common - 2.4.0 + ${junixsocket.version} test @@ -306,6 +306,7 @@ 17 17 + 17 From 17d07eb50145645b921d19aee33647239531f291 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Sat, 18 Jun 2022 16:44:46 +0200 Subject: [PATCH 13/25] test: add more tests --- .../cloud/spanner/pgadapter/ITJdbcTest.java | 93 ++++++++++--------- .../spanner/pgadapter/JdbcMockServerTest.java | 51 ++++++++++ .../parsers/UnspecifiedParserTest.java | 51 ++++++++++ 3 files changed, 152 insertions(+), 43 deletions(-) create mode 100644 src/test/java/com/google/cloud/spanner/pgadapter/parsers/UnspecifiedParserTest.java diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcTest.java index 2157d1692..801c4e10f 100644 --- a/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcTest.java +++ b/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcTest.java @@ -346,52 +346,59 @@ public void testUpdateWithParameters() throws SQLException { @Test public void testNullValues() throws SQLException { try (Connection connection = DriverManager.getConnection(getConnectionUrl())) { - try (PreparedStatement statement = - connection.prepareStatement( - "insert into all_types " - + "(col_bigint, col_bool, col_bytea, col_float8, col_int, col_numeric, col_timestamptz, col_date, col_varchar) " - + "values (?, ?, ?, ?, ?, ?, ?, ?, ?)")) { - int index = 0; - statement.setLong(++index, 2); - statement.setNull(++index, Types.BOOLEAN); - statement.setNull(++index, Types.BINARY); - statement.setNull(++index, Types.DOUBLE); - statement.setNull(++index, Types.INTEGER); - statement.setNull(++index, Types.NUMERIC); - statement.setNull(++index, Types.TIMESTAMP_WITH_TIMEZONE); - statement.setNull(++index, Types.DATE); - statement.setNull(++index, Types.VARCHAR); - - assertEquals(1, statement.executeUpdate()); - } - - try (ResultSet resultSet = - connection.createStatement().executeQuery("select * from all_types where col_bigint=2")) { - assertTrue(resultSet.next()); + for (boolean typed : new boolean[] {true, false}) { + try (PreparedStatement statement = + connection.prepareStatement( + "insert into all_types " + + "(col_bigint, col_bool, col_bytea, col_float8, col_int, col_numeric, col_timestamptz, col_date, col_varchar) " + + "values (?, ?, ?, ?, ?, ?, ?, ?, ?)")) { + int index = 0; + statement.setLong(++index, 2); + statement.setNull(++index, typed ? Types.BOOLEAN : Types.NULL); + statement.setNull(++index, typed ? Types.BINARY : Types.NULL); + statement.setNull(++index, typed ? Types.DOUBLE : Types.NULL); + statement.setNull(++index, typed ? Types.INTEGER : Types.NULL); + statement.setNull(++index, typed ? Types.NUMERIC : Types.NULL); + statement.setNull(++index, typed ? Types.TIMESTAMP_WITH_TIMEZONE : Types.NULL); + statement.setNull(++index, typed ? Types.DATE : Types.NULL); + statement.setNull(++index, typed ? Types.VARCHAR : Types.NULL); + + assertEquals(1, statement.executeUpdate()); + } - int index = 0; - assertEquals(2, resultSet.getLong(++index)); + try (ResultSet resultSet = + connection + .createStatement() + .executeQuery("select * from all_types where col_bigint=2")) { + assertTrue(resultSet.next()); - // Note: JDBC returns the zero-value for primitive types if the value is NULL, and you have - // to call wasNull() to determine whether the value was NULL or zero. - assertFalse(resultSet.getBoolean(++index)); - assertTrue(resultSet.wasNull()); - assertNull(resultSet.getBytes(++index)); - assertTrue(resultSet.wasNull()); - assertEquals(0d, resultSet.getDouble(++index), 0.0d); - assertTrue(resultSet.wasNull()); - assertEquals(0, resultSet.getInt(++index)); - assertTrue(resultSet.wasNull()); - assertNull(resultSet.getBigDecimal(++index)); - assertTrue(resultSet.wasNull()); - assertNull(resultSet.getTimestamp(++index)); - assertTrue(resultSet.wasNull()); - assertNull(resultSet.getDate(++index)); - assertTrue(resultSet.wasNull()); - assertNull(resultSet.getString(++index)); - assertTrue(resultSet.wasNull()); + int index = 0; + assertEquals(2, resultSet.getLong(++index)); + + // Note: JDBC returns the zero-value for primitive types if the value is NULL, and you + // have to call wasNull() to determine whether the value was NULL or zero. + assertFalse(resultSet.getBoolean(++index)); + assertTrue(resultSet.wasNull()); + assertNull(resultSet.getBytes(++index)); + assertTrue(resultSet.wasNull()); + assertEquals(0d, resultSet.getDouble(++index), 0.0d); + assertTrue(resultSet.wasNull()); + assertEquals(0, resultSet.getInt(++index)); + assertTrue(resultSet.wasNull()); + assertNull(resultSet.getBigDecimal(++index)); + assertTrue(resultSet.wasNull()); + assertNull(resultSet.getTimestamp(++index)); + assertTrue(resultSet.wasNull()); + assertNull(resultSet.getDate(++index)); + assertTrue(resultSet.wasNull()); + assertNull(resultSet.getString(++index)); + assertTrue(resultSet.wasNull()); - assertFalse(resultSet.next()); + assertFalse(resultSet.next()); + } + assertEquals( + 1, + connection.createStatement().executeUpdate("delete from all_types where col_bigint=2")); } } } diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java index 9783160f6..cf4919b1d 100644 --- a/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java +++ b/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java @@ -416,6 +416,57 @@ public void testNullValues() throws SQLException { assertEquals(2, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class)); } + @Test + public void testInsertUntypedNulls() throws SQLException { + mockSpanner.putStatementResult( + StatementResult.update( + Statement.newBuilder( + "insert into all_types " + + "(col_bigint, col_bool, col_bytea, col_float8, col_int, col_numeric, col_timestamptz, col_date, col_varchar) " + + "values ($1, $2, $3, $4, $5, $6, $7, $8, $9)") + .bind("p1") + .to(2L) + .bind("p2") + .to((com.google.cloud.spanner.Value) null) + .bind("p3") + .to((com.google.cloud.spanner.Value) null) + .bind("p4") + .to((com.google.cloud.spanner.Value) null) + .bind("p5") + .to((com.google.cloud.spanner.Value) null) + .bind("p6") + .to((com.google.cloud.spanner.Value) null) + .bind("p7") + .to((com.google.cloud.spanner.Value) null) + .bind("p8") + .to((com.google.cloud.spanner.Value) null) + .bind("p9") + .to((com.google.cloud.spanner.Value) null) + .build(), + 1L)); + + try (Connection connection = DriverManager.getConnection(createUrl())) { + try (PreparedStatement statement = + connection.prepareStatement( + "insert into all_types " + + "(col_bigint, col_bool, col_bytea, col_float8, col_int, col_numeric, col_timestamptz, col_date, col_varchar) " + + "values (?, ?, ?, ?, ?, ?, ?, ?, ?)")) { + int index = 0; + statement.setLong(++index, 2); + statement.setNull(++index, Types.NULL); + statement.setNull(++index, Types.NULL); + statement.setNull(++index, Types.NULL); + statement.setNull(++index, Types.NULL); + statement.setNull(++index, Types.NULL); + statement.setNull(++index, Types.NULL); + statement.setNull(++index, Types.NULL); + statement.setNull(++index, Types.NULL); + + assertEquals(1, statement.executeUpdate()); + } + } + } + @Test public void testDescribeQueryWithNonExistingTable() throws SQLException { String sql = "select * from non_existing_table where id=$1"; diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/parsers/UnspecifiedParserTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/parsers/UnspecifiedParserTest.java new file mode 100644 index 000000000..455db53f6 --- /dev/null +++ b/src/test/java/com/google/cloud/spanner/pgadapter/parsers/UnspecifiedParserTest.java @@ -0,0 +1,51 @@ +// Copyright 2022 Google LLC +// +// Licensed 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 com.google.cloud.spanner.pgadapter.parsers; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; + +import com.google.cloud.spanner.Value; +import com.google.cloud.spanner.pgadapter.parsers.Parser.FormatCode; +import java.nio.charset.StandardCharsets; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.postgresql.core.Oid; + +@RunWith(JUnit4.class) +public class UnspecifiedParserTest { + + @Test + public void testCreateUnspecifiedParser() { + Parser parser = Parser.create(new byte[] {}, Oid.UNSPECIFIED, FormatCode.TEXT); + + assertEquals(UnspecifiedParser.class, parser.getClass()); + } + + @Test + public void testParseNonNull() { + Parser parser = + Parser.create( + "unspecified value".getBytes(StandardCharsets.UTF_8), Oid.UNSPECIFIED, FormatCode.TEXT); + + assertEquals( + Value.untyped( + com.google.protobuf.Value.newBuilder().setStringValue("unspecified value").build()), + parser.getItem()); + assertEquals("unspecified value", parser.stringParse()); + assertArrayEquals("unspecified value".getBytes(StandardCharsets.UTF_8), parser.binaryParse()); + } +} From b426d22a2b532e06620940d54bcb2601ec8e8c14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Fri, 24 Jun 2022 19:45:47 +0200 Subject: [PATCH 14/25] perf: try to increase buffer size --- .../cloud/spanner/pgadapter/Java17ServerSocketFactory.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/Java17ServerSocketFactory.java b/src/main/java/com/google/cloud/spanner/pgadapter/Java17ServerSocketFactory.java index 811476199..30a02740b 100644 --- a/src/main/java/com/google/cloud/spanner/pgadapter/Java17ServerSocketFactory.java +++ b/src/main/java/com/google/cloud/spanner/pgadapter/Java17ServerSocketFactory.java @@ -14,6 +14,8 @@ package com.google.cloud.spanner.pgadapter; +import static java.net.StandardSocketOptions.SO_RCVBUF; + import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata; import java.io.File; import java.io.IOException; @@ -69,6 +71,7 @@ public ServerSocketChannel creatUnixDomainSocketChannel(OptionsMetadata options, UnixDomainSocketAddress address = UnixDomainSocketAddress.of(path); ServerSocketChannel domainSocketChannel = ServerSocketChannel.open(StandardProtocolFamily.UNIX); domainSocketChannel.configureBlocking(true); + domainSocketChannel.setOption(SO_RCVBUF, 65536); domainSocketChannel.bind(address, options.getMaxBacklog()); return domainSocketChannel; From 4b279b74c1d1b7d8c8ce04fa4745eb615f703f3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Sat, 25 Jun 2022 14:24:58 +0200 Subject: [PATCH 15/25] build: add automated ycsb run --- .ci/run-ycsb.sh | 31 +++++++++++++++++++++++++++ .ci/ycsb-setup.sh | 54 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100755 .ci/run-ycsb.sh create mode 100644 .ci/ycsb-setup.sh diff --git a/.ci/run-ycsb.sh b/.ci/run-ycsb.sh new file mode 100755 index 000000000..306ff574e --- /dev/null +++ b/.ci/run-ycsb.sh @@ -0,0 +1,31 @@ +for OPERATION_COUNT in 1000 +do + for THREADS in 1 5 + do + for BATCH_SIZE in 1 5 + do + ./bin/ycsb run jdbc -P workloads/workloadd \ + -threads ${THREADS} \ + -p operationcount=${OPERATION_COUNT} \ + -p jdbc.batchupdateapi=true \ + -P uds.properties -cp "jdbc-binding/lib/*" > output.txt + + RUNTIME=$(sed -En 's/\[OVERALL\], RunTime\(ms\), (.+)$/\1/p' output.txt) + THROUGHPUT=$(sed -En 's/\[OVERALL\], Throughput\(ops\/sec\), (.+)$/\1/p' output.txt) + READ_AVG=$(sed -En 's/\[READ\], AverageLatency\(us\), (.+)$/\1/p' output.txt) + READ_P95=$(sed -En 's/\[READ\], 95thPercentileLatency\(us\), (.+)$/\1/p' output.txt) + READ_P99=$(sed -En 's/\[READ\], 99thPercentileLatency\(us\), (.+)$/\1/p' output.txt) + INSERT_AVG=$(sed -En 's/\[INSERT\], AverageLatency\(us\), (.+)$/\1/p' output.txt) + INSERT_P95=$(sed -En 's/\[INSERT\], 95thPercentileLatency\(us\), (.+)$/\1/p' output.txt) + INSERT_P99=$(sed -En 's/\[INSERT\], 99thPercentileLatency\(us\), (.+)$/\1/p' output.txt) + + psql -h /tmp -p 5433 -c " + insert into run (deployment, workload, threads, batch_size, operation_count, run_time, throughput, + read_avg, read_p95, read_p99, insert_avg, insert_p95, insert_p99) + values ('jar', 'd', ${THREADS}, ${BATCH_SIZE}, ${OPERATION_COUNT}, ${RUNTIME}, ${THROUGHPUT}, + ${READ_AVG}, ${READ_P95}, ${READ_P99}, ${INSERT_AVG}, ${INSERT_P95}, ${INSERT_P99});" + + psql -h /tmp -p 5433 -c "set spanner.autocommit_dml_mode='partitioned_non_atomic'; delete from usertable;" + done + done +done diff --git a/.ci/ycsb-setup.sh b/.ci/ycsb-setup.sh new file mode 100644 index 000000000..ced4b4260 --- /dev/null +++ b/.ci/ycsb-setup.sh @@ -0,0 +1,54 @@ + +# native-image -J-Xmx10g -H:IncludeResources=".*metadata.*json$" -jar pgadapter.jar --no-fallback + + +mvn clean package -Passembly -DskipTests +cd target/pgadapter +java -jar pgadapter.jar -p appdev-soda-spanner-staging -i knut-test-ycsb -d ycsb &> pgadapter.log & +psql -h /tmp -c "CREATE TABLE usertable ( + YCSB_KEY VARCHAR(255) PRIMARY KEY, + FIELD0 TEXT, FIELD1 TEXT, + FIELD2 TEXT, FIELD3 TEXT, + FIELD4 TEXT, FIELD5 TEXT, + FIELD6 TEXT, FIELD7 TEXT, + FIELD8 TEXT, FIELD9 TEXT + );" +psql -h /tmp -c "CREATE TABLE run ( + deployment varchar, + workload varchar, + threads bigint, + batch_size bigint, + operation_count bigint, + run_time bigint, + throughput float, + read_avg float, + read_p95 float, + read_p99 float, + insert_avg float, + insert_p95 float, + insert_p99 float, + primary key (deployment, workload, threads, batch_size, operation_count) + );" +cd ~ +git clone git@github.com:brianfrankcooper/YCSB.git +cd YCSB + +mkdir jdbc-binding +cd jdbc-binding +mkdir lib +cd lib +wget https://repo1.maven.org/maven2/org/postgresql/postgresql/42.4.0/postgresql-42.4.0.jar +wget https://repo1.maven.org/maven2/com/kohlschutter/junixsocket/junixsocket-common/2.5.0/junixsocket-common-2.5.0.jar +wget https://repo1.maven.org/maven2/com/kohlschutter/junixsocket/junixsocket-native-common/2.5.0/junixsocket-native-common-2.5.0.jar +cd ../.. + +cat <> uds.properties +db.driver=org.postgresql.Driver +db.url=jdbc:postgresql://localhost/ycsb?socketFactory=org.newsclub.net.unix.AFUNIXSocketFactory\$FactoryArg&socketFactoryArg=/tmp/.s.PGSQL.5432 +db.user=admin +db.passwd=admin +EOT + +./bin/ycsb run jdbc -P workloads/workloadd -threads 1 -p operationcount=1000 -P uds.properties -cp "jdbc-binding/lib/*" + +psql -h /tmp -c "set spanner.autocommit_dml_mode='partitioned_non_atomic'; delete from usertable;" From 7d9d1a61552e14a9f45db5e9141c0da50a14c968 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Mon, 27 Jun 2022 12:23:43 +0200 Subject: [PATCH 16/25] build: automate ycsb runs --- .ci/run-ycsb.sh | 7 ++++--- .ci/ycsb-setup.sh | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/.ci/run-ycsb.sh b/.ci/run-ycsb.sh index 306ff574e..059c21d8d 100755 --- a/.ci/run-ycsb.sh +++ b/.ci/run-ycsb.sh @@ -4,11 +4,12 @@ do do for BATCH_SIZE in 1 5 do - ./bin/ycsb run jdbc -P workloads/workloadd \ + ./bin/ycsb ${YCSB_COMMAND} jdbc -P workloads/${WORKLOAD} \ -threads ${THREADS} \ -p operationcount=${OPERATION_COUNT} \ -p jdbc.batchupdateapi=true \ - -P uds.properties -cp "jdbc-binding/lib/*" > output.txt + -P ${YCSB_PROPERTY_FILE} \ + -cp "jdbc-binding/lib/*" > output.txt RUNTIME=$(sed -En 's/\[OVERALL\], RunTime\(ms\), (.+)$/\1/p' output.txt) THROUGHPUT=$(sed -En 's/\[OVERALL\], Throughput\(ops\/sec\), (.+)$/\1/p' output.txt) @@ -22,7 +23,7 @@ do psql -h /tmp -p 5433 -c " insert into run (deployment, workload, threads, batch_size, operation_count, run_time, throughput, read_avg, read_p95, read_p99, insert_avg, insert_p95, insert_p99) - values ('jar', 'd', ${THREADS}, ${BATCH_SIZE}, ${OPERATION_COUNT}, ${RUNTIME}, ${THROUGHPUT}, + values (${DEPLOYMENT}, ${WORKLOAD}, ${THREADS}, ${BATCH_SIZE}, ${OPERATION_COUNT}, ${RUNTIME}, ${THROUGHPUT}, ${READ_AVG}, ${READ_P95}, ${READ_P99}, ${INSERT_AVG}, ${INSERT_P95}, ${INSERT_P99});" psql -h /tmp -p 5433 -c "set spanner.autocommit_dml_mode='partitioned_non_atomic'; delete from usertable;" diff --git a/.ci/ycsb-setup.sh b/.ci/ycsb-setup.sh index ced4b4260..5bc33209f 100644 --- a/.ci/ycsb-setup.sh +++ b/.ci/ycsb-setup.sh @@ -49,6 +49,7 @@ db.user=admin db.passwd=admin EOT -./bin/ycsb run jdbc -P workloads/workloadd -threads 1 -p operationcount=1000 -P uds.properties -cp "jdbc-binding/lib/*" +export YCSB_PROPERTY_FILE=uds.properties +export YCSB_COMMAND=run +export WORKLOAD=workloadd -psql -h /tmp -c "set spanner.autocommit_dml_mode='partitioned_non_atomic'; delete from usertable;" From 45385c456b7be1070250f0f59239f7f72b01940a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 29 Jun 2022 11:45:11 +0200 Subject: [PATCH 17/25] build: update ycsb script --- .ci/run-ycsb.sh | 6 +++--- .ci/ycsb-setup.sh | 25 +++++++++++++++++++------ 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/.ci/run-ycsb.sh b/.ci/run-ycsb.sh index 059c21d8d..3c14ecaa0 100755 --- a/.ci/run-ycsb.sh +++ b/.ci/run-ycsb.sh @@ -20,13 +20,13 @@ do INSERT_P95=$(sed -En 's/\[INSERT\], 95thPercentileLatency\(us\), (.+)$/\1/p' output.txt) INSERT_P99=$(sed -En 's/\[INSERT\], 99thPercentileLatency\(us\), (.+)$/\1/p' output.txt) - psql -h /tmp -p 5433 -c " + psql -h /tmp -p ${PORT} -c " insert into run (deployment, workload, threads, batch_size, operation_count, run_time, throughput, read_avg, read_p95, read_p99, insert_avg, insert_p95, insert_p99) - values (${DEPLOYMENT}, ${WORKLOAD}, ${THREADS}, ${BATCH_SIZE}, ${OPERATION_COUNT}, ${RUNTIME}, ${THROUGHPUT}, + values ('${DEPLOYMENT}', '${WORKLOAD}', ${THREADS}, ${BATCH_SIZE}, ${OPERATION_COUNT}, ${RUNTIME}, ${THROUGHPUT}, ${READ_AVG}, ${READ_P95}, ${READ_P99}, ${INSERT_AVG}, ${INSERT_P95}, ${INSERT_P99});" - psql -h /tmp -p 5433 -c "set spanner.autocommit_dml_mode='partitioned_non_atomic'; delete from usertable;" + psql -h /tmp -p ${PORT} -c "set spanner.autocommit_dml_mode='partitioned_non_atomic'; delete from usertable;" done done done diff --git a/.ci/ycsb-setup.sh b/.ci/ycsb-setup.sh index 5bc33209f..4d8e06c80 100644 --- a/.ci/ycsb-setup.sh +++ b/.ci/ycsb-setup.sh @@ -1,19 +1,21 @@ # native-image -J-Xmx10g -H:IncludeResources=".*metadata.*json$" -jar pgadapter.jar --no-fallback +export PORT=5433 - +git clone git@github.com:GoogleCloudPlatform/pgadapter.git +cd pgadapter mvn clean package -Passembly -DskipTests cd target/pgadapter -java -jar pgadapter.jar -p appdev-soda-spanner-staging -i knut-test-ycsb -d ycsb &> pgadapter.log & -psql -h /tmp -c "CREATE TABLE usertable ( +java -jar pgadapter.jar -p appdev-soda-spanner-staging -i knut-test-ycsb -d ycsb -s ${PORT} &> pgadapter.log & +psql -h /tmp -p ${PORT} -c "CREATE TABLE IF NOT EXISTS usertable ( YCSB_KEY VARCHAR(255) PRIMARY KEY, FIELD0 TEXT, FIELD1 TEXT, FIELD2 TEXT, FIELD3 TEXT, FIELD4 TEXT, FIELD5 TEXT, FIELD6 TEXT, FIELD7 TEXT, FIELD8 TEXT, FIELD9 TEXT - );" -psql -h /tmp -c "CREATE TABLE run ( + ); + CREATE TABLE IF NOT EXISTS run ( deployment varchar, workload varchar, threads bigint, @@ -42,14 +44,25 @@ wget https://repo1.maven.org/maven2/com/kohlschutter/junixsocket/junixsocket-com wget https://repo1.maven.org/maven2/com/kohlschutter/junixsocket/junixsocket-native-common/2.5.0/junixsocket-native-common-2.5.0.jar cd ../.. +psql -h /tmp -p ${PORT} -c "delete from run;" +psql -h /tmp -p ${PORT} -c "set spanner.autocommit_dml_mode='partitioned_non_atomic'; delete from usertable;" +pkill -f pgadapter + cat <> uds.properties db.driver=org.postgresql.Driver -db.url=jdbc:postgresql://localhost/ycsb?socketFactory=org.newsclub.net.unix.AFUNIXSocketFactory\$FactoryArg&socketFactoryArg=/tmp/.s.PGSQL.5432 +db.url=jdbc:postgresql://localhost/ycsb?socketFactory=org.newsclub.net.unix.AFUNIXSocketFactory\$FactoryArg&socketFactoryArg=/tmp/.s.PGSQL.${PORT} db.user=admin db.passwd=admin EOT +cd ../pgadapter +mvn clean package -Passembly -DskipTests +cd target/pgadapter +java -jar pgadapter.jar -p appdev-soda-spanner-staging -i knut-test-ycsb -d ycsb -s ${PORT} &> pgadapter.log & +export DEPLOYMENT=jar-java8 export YCSB_PROPERTY_FILE=uds.properties export YCSB_COMMAND=run export WORKLOAD=workloadd +cd ../../../YCSB +../pgadapter/.ci/run-ycsb.sh From c1ac96b04a1ef37b47819bb839bf3e619aa9d1a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 29 Jun 2022 12:03:00 +0200 Subject: [PATCH 18/25] fix: use 0 instead of NaN --- .ci/run-ycsb.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.ci/run-ycsb.sh b/.ci/run-ycsb.sh index 3c14ecaa0..b303b806f 100755 --- a/.ci/run-ycsb.sh +++ b/.ci/run-ycsb.sh @@ -20,6 +20,13 @@ do INSERT_P95=$(sed -En 's/\[INSERT\], 95thPercentileLatency\(us\), (.+)$/\1/p' output.txt) INSERT_P99=$(sed -En 's/\[INSERT\], 99thPercentileLatency\(us\), (.+)$/\1/p' output.txt) + if [ "$READ_AVG" == "NaN" ]; then $READ_AVG=0; fi + if [ "$READ_P95" == "NaN" ]; then $READ_P95=0; fi + if [ "$READ_P99" == "NaN" ]; then $READ_P99=0; fi + if [ "$INSERT_AVG" == "NaN" ]; then $INSERT_AVG=0; fi + if [ "$INSERT_P95" == "NaN" ]; then $INSERT_P95=0; fi + if [ "$INSERT_P99" == "NaN" ]; then $INSERT_P99=0; fi + psql -h /tmp -p ${PORT} -c " insert into run (deployment, workload, threads, batch_size, operation_count, run_time, throughput, read_avg, read_p95, read_p99, insert_avg, insert_p95, insert_p99) From 3f9198312f2250f13603b2e6f31c7eb3b4452032 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 29 Jun 2022 12:08:32 +0200 Subject: [PATCH 19/25] fix: remove $ from set command --- .ci/run-ycsb.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.ci/run-ycsb.sh b/.ci/run-ycsb.sh index b303b806f..59d0cd577 100755 --- a/.ci/run-ycsb.sh +++ b/.ci/run-ycsb.sh @@ -20,12 +20,12 @@ do INSERT_P95=$(sed -En 's/\[INSERT\], 95thPercentileLatency\(us\), (.+)$/\1/p' output.txt) INSERT_P99=$(sed -En 's/\[INSERT\], 99thPercentileLatency\(us\), (.+)$/\1/p' output.txt) - if [ "$READ_AVG" == "NaN" ]; then $READ_AVG=0; fi - if [ "$READ_P95" == "NaN" ]; then $READ_P95=0; fi - if [ "$READ_P99" == "NaN" ]; then $READ_P99=0; fi - if [ "$INSERT_AVG" == "NaN" ]; then $INSERT_AVG=0; fi - if [ "$INSERT_P95" == "NaN" ]; then $INSERT_P95=0; fi - if [ "$INSERT_P99" == "NaN" ]; then $INSERT_P99=0; fi + if [ "$READ_AVG" == "NaN" ]; then READ_AVG=0; fi + if [ "$READ_P95" == "NaN" ]; then READ_P95=0; fi + if [ "$READ_P99" == "NaN" ]; then READ_P99=0; fi + if [ "$INSERT_AVG" == "NaN" ]; then INSERT_AVG=0; fi + if [ "$INSERT_P95" == "NaN" ]; then INSERT_P95=0; fi + if [ "$INSERT_P99" == "NaN" ]; then INSERT_P99=0; fi psql -h /tmp -p ${PORT} -c " insert into run (deployment, workload, threads, batch_size, operation_count, run_time, throughput, From 3a47142cb19bd009e03fb1298ca0e17066cbde05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 29 Jun 2022 12:18:33 +0200 Subject: [PATCH 20/25] fix: actually use batch size --- .ci/run-ycsb.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/.ci/run-ycsb.sh b/.ci/run-ycsb.sh index 59d0cd577..db2f8d5e0 100755 --- a/.ci/run-ycsb.sh +++ b/.ci/run-ycsb.sh @@ -8,6 +8,7 @@ do -threads ${THREADS} \ -p operationcount=${OPERATION_COUNT} \ -p jdbc.batchupdateapi=true \ + -p db.batchsize=${BATCH_SIZE} \ -P ${YCSB_PROPERTY_FILE} \ -cp "jdbc-binding/lib/*" > output.txt From b9dced6046662ee5b7503bcaa44751dccd2548a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 29 Jun 2022 14:05:49 +0200 Subject: [PATCH 21/25] feat: add additional benchmark runs --- .ci/run-ycsb.sh | 8 +++---- .ci/ycsb-setup.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/.ci/run-ycsb.sh b/.ci/run-ycsb.sh index db2f8d5e0..647f05b40 100755 --- a/.ci/run-ycsb.sh +++ b/.ci/run-ycsb.sh @@ -1,8 +1,8 @@ -for OPERATION_COUNT in 1000 +for OPERATION_COUNT in 5000 10000 50000 do - for THREADS in 1 5 + for THREADS in 1 5 20 50 do - for BATCH_SIZE in 1 5 + for BATCH_SIZE in ${BATCH_SIZES} do ./bin/ycsb ${YCSB_COMMAND} jdbc -P workloads/${WORKLOAD} \ -threads ${THREADS} \ @@ -34,7 +34,7 @@ do values ('${DEPLOYMENT}', '${WORKLOAD}', ${THREADS}, ${BATCH_SIZE}, ${OPERATION_COUNT}, ${RUNTIME}, ${THROUGHPUT}, ${READ_AVG}, ${READ_P95}, ${READ_P99}, ${INSERT_AVG}, ${INSERT_P95}, ${INSERT_P99});" - psql -h /tmp -p ${PORT} -c "set spanner.autocommit_dml_mode='partitioned_non_atomic'; delete from usertable;" + if [ "$YCSB_DELETE_AFTER" == "YES" ]; then psql -h /tmp -p ${PORT} -c "set spanner.autocommit_dml_mode='partitioned_non_atomic'; delete from usertable;"; fi done done done diff --git a/.ci/ycsb-setup.sh b/.ci/ycsb-setup.sh index 4d8e06c80..3e22ed6fc 100644 --- a/.ci/ycsb-setup.sh +++ b/.ci/ycsb-setup.sh @@ -55,14 +55,65 @@ db.user=admin db.passwd=admin EOT +cat <> tcp.properties +db.driver=org.postgresql.Driver +db.url=jdbc:postgresql://localhost:${PORT} +db.user=admin +db.passwd=admin +EOT + +# Run Unix Domain Socket Java 8 cd ../pgadapter mvn clean package -Passembly -DskipTests cd target/pgadapter java -jar pgadapter.jar -p appdev-soda-spanner-staging -i knut-test-ycsb -d ycsb -s ${PORT} &> pgadapter.log & -export DEPLOYMENT=jar-java8 +export DEPLOYMENT=jar-java8-uds export YCSB_PROPERTY_FILE=uds.properties export YCSB_COMMAND=run export WORKLOAD=workloadd - +export YCSB_DELETE_AFTER=YES +export BATCH_SIZES="1 5 20 50 100" cd ../../../YCSB ../pgadapter/.ci/run-ycsb.sh +pkill -f pgadapter +# Run Unix Domain Socket Java 17 +cd ../pgadapter +mvn clean package -Passembly -Pnative-image -DskipTests +cd target/pgadapter +java -jar pgadapter.jar -p appdev-soda-spanner-staging -i knut-test-ycsb -d ycsb -s ${PORT} &> pgadapter.log & +export DEPLOYMENT=jar-java17-uds +export YCSB_PROPERTY_FILE=uds.properties +export YCSB_COMMAND=run +export WORKLOAD=workloadd +export YCSB_DELETE_AFTER=YES +export BATCH_SIZES="1 5 20 50 100" +cd ../../../YCSB +../pgadapter/.ci/run-ycsb.sh +pkill -f pgadapter +# Run TCP Java 8 +cd ../pgadapter +mvn clean package -Passembly -DskipTests +cd target/pgadapter +java -jar pgadapter.jar -p appdev-soda-spanner-staging -i knut-test-ycsb -d ycsb -s ${PORT} &> pgadapter.log & +export DEPLOYMENT=jar-java8-tcp +export YCSB_PROPERTY_FILE=tcp.properties +export YCSB_COMMAND=run +export WORKLOAD=workloadd +export YCSB_DELETE_AFTER=YES +export BATCH_SIZES="1 5 20 50 100" +cd ../../../YCSB +../pgadapter/.ci/run-ycsb.sh +pkill -f pgadapter +cd ../pgadapter +mvn clean package -Passembly -Pnative-image -DskipTests +cd target/pgadapter +java -jar pgadapter.jar -p appdev-soda-spanner-staging -i knut-test-ycsb -d ycsb -s ${PORT} &> pgadapter.log & +export DEPLOYMENT=jar-java17-tcp +export YCSB_PROPERTY_FILE=tcp.properties +export YCSB_COMMAND=run +export WORKLOAD=workloadd +export YCSB_DELETE_AFTER=YES +export BATCH_SIZES="1 5 20 50 100" +cd ../../../YCSB +../pgadapter/.ci/run-ycsb.sh +pkill -f pgadapter From 13d4f018c5fc68f6be0c2fcbb2ad8ba0f1d808db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 29 Jun 2022 15:56:10 +0200 Subject: [PATCH 22/25] fix: remove files before appending --- .ci/ycsb-setup.sh | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/.ci/ycsb-setup.sh b/.ci/ycsb-setup.sh index 3e22ed6fc..050784d5c 100644 --- a/.ci/ycsb-setup.sh +++ b/.ci/ycsb-setup.sh @@ -31,6 +31,8 @@ psql -h /tmp -p ${PORT} -c "CREATE TABLE IF NOT EXISTS usertable ( insert_p99 float, primary key (deployment, workload, threads, batch_size, operation_count) );" +pkill -f pgadapter + cd ~ git clone git@github.com:brianfrankcooper/YCSB.git cd YCSB @@ -42,12 +44,18 @@ cd lib wget https://repo1.maven.org/maven2/org/postgresql/postgresql/42.4.0/postgresql-42.4.0.jar wget https://repo1.maven.org/maven2/com/kohlschutter/junixsocket/junixsocket-common/2.5.0/junixsocket-common-2.5.0.jar wget https://repo1.maven.org/maven2/com/kohlschutter/junixsocket/junixsocket-native-common/2.5.0/junixsocket-native-common-2.5.0.jar -cd ../.. +cd ~/pgadapter +mvn package -Passembly -DskipTests +cd target/pgadapter +java -jar pgadapter.jar -p appdev-soda-spanner-staging -i knut-test-ycsb -d ycsb -s ${PORT} &> pgadapter.log & +sleep 1 psql -h /tmp -p ${PORT} -c "delete from run;" psql -h /tmp -p ${PORT} -c "set spanner.autocommit_dml_mode='partitioned_non_atomic'; delete from usertable;" pkill -f pgadapter +cd ~/YCSB +rm uds.properties cat <> uds.properties db.driver=org.postgresql.Driver db.url=jdbc:postgresql://localhost/ycsb?socketFactory=org.newsclub.net.unix.AFUNIXSocketFactory\$FactoryArg&socketFactoryArg=/tmp/.s.PGSQL.${PORT} @@ -55,9 +63,10 @@ db.user=admin db.passwd=admin EOT +rm tcp.properties cat <> tcp.properties db.driver=org.postgresql.Driver -db.url=jdbc:postgresql://localhost:${PORT} +db.url=jdbc:postgresql://localhost:${PORT}/ycsb db.user=admin db.passwd=admin EOT From cc9ad505001ef17c10b2eb3dad740f1109e73b0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 29 Jun 2022 20:35:37 +0200 Subject: [PATCH 23/25] feat: add run command for native image --- .ci/ycsb-setup.sh | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/.ci/ycsb-setup.sh b/.ci/ycsb-setup.sh index 050784d5c..aa6eb2d71 100644 --- a/.ci/ycsb-setup.sh +++ b/.ci/ycsb-setup.sh @@ -1,5 +1,6 @@ # native-image -J-Xmx10g -H:IncludeResources=".*metadata.*json$" -jar pgadapter.jar --no-fallback +export GOOGLE_APPLICATION_CREDENTIALS= export PORT=5433 git clone git@github.com:GoogleCloudPlatform/pgadapter.git @@ -85,6 +86,7 @@ export BATCH_SIZES="1 5 20 50 100" cd ../../../YCSB ../pgadapter/.ci/run-ycsb.sh pkill -f pgadapter + # Run Unix Domain Socket Java 17 cd ../pgadapter mvn clean package -Passembly -Pnative-image -DskipTests @@ -99,6 +101,7 @@ export BATCH_SIZES="1 5 20 50 100" cd ../../../YCSB ../pgadapter/.ci/run-ycsb.sh pkill -f pgadapter + # Run TCP Java 8 cd ../pgadapter mvn clean package -Passembly -DskipTests @@ -113,6 +116,8 @@ export BATCH_SIZES="1 5 20 50 100" cd ../../../YCSB ../pgadapter/.ci/run-ycsb.sh pkill -f pgadapter + +# Run TCP Java 17 cd ../pgadapter mvn clean package -Passembly -Pnative-image -DskipTests cd target/pgadapter @@ -126,3 +131,34 @@ export BATCH_SIZES="1 5 20 50 100" cd ../../../YCSB ../pgadapter/.ci/run-ycsb.sh pkill -f pgadapter + +# Run native image Unix domain sockets +# This assumes that the native image has been uploaded manually to the VM. +#cd ../pgadapter +#mvn clean package -Passembly -Pnative-image -DskipTests +#cd target/pgadapter +#native-image -J-Xmx10g -H:IncludeResources=".*metadata.*json$" -jar pgadapter.jar --no-fallback +cd ~/native +./pgadapter -p appdev-soda-spanner-staging -i knut-test-ycsb -d ycsb -s ${PORT} &> pgadapter.log & +export DEPLOYMENT=native-uds +export YCSB_PROPERTY_FILE=uds.properties +export YCSB_COMMAND=run +export WORKLOAD=workloadd +export YCSB_DELETE_AFTER=YES +export BATCH_SIZES="1 5 20 50 100" +cd ~/YCSB +../pgadapter/.ci/run-ycsb.sh +pkill -f pgadapter + +# Run native image TCP +cd ~/native +./pgadapter -p appdev-soda-spanner-staging -i knut-test-ycsb -d ycsb -s ${PORT} &> pgadapter.log & +export DEPLOYMENT=native-tcp +export YCSB_PROPERTY_FILE=tcp.properties +export YCSB_COMMAND=run +export WORKLOAD=workloadd +export YCSB_DELETE_AFTER=YES +export BATCH_SIZES="1 5 20 50 100" +cd ~/YCSB +../pgadapter/.ci/run-ycsb.sh +pkill -f pgadapter From 9bdea78fb2a17b237b6345593ac91e553a96b0fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Thu, 30 Jun 2022 06:30:23 +0200 Subject: [PATCH 24/25] test: fix assertion as connection can be closed by error --- .../google/cloud/spanner/pgadapter/ITJdbcTest.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcTest.java index b34336006..fafc85378 100644 --- a/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcTest.java +++ b/src/test/java/com/google/cloud/spanner/pgadapter/ITJdbcTest.java @@ -491,11 +491,14 @@ public void testCopyIn_Large_FailsWhenAtomic() throws SQLException { copyManager.copyIn( "copy all_types from stdin;", new FileInputStream("./src/test/resources/all_types_data.txt"))); - assertEquals( - "ERROR: FAILED_PRECONDITION: Record count: 2001 has exceeded the limit: 2000.\n\n" - + "The number of mutations per record is equal to the number of columns in the record plus the number of indexed columns in the record. The maximum number of mutations in one transaction is 20000.\n\n" - + "Execute `SET AUTOCOMMIT_DML_MODE='PARTITIONED_NON_ATOMIC'` before executing a large COPY operation to instruct PGAdapter to automatically break large transactions into multiple smaller. This will make the COPY operation non-atomic.\n\n", - exception.getMessage()); + assertTrue( + exception.getMessage(), + exception + .getMessage() + .contains("FAILED_PRECONDITION: Record count: 2001 has exceeded the limit: 2000.") + || exception + .getMessage() + .contains("Database connection failed when canceling copy operation")); } // Verify that the table is still empty. From 591e90ddba3528061b88d4d8fcadab6d4c9a9fc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Thu, 30 Jun 2022 14:27:49 +0200 Subject: [PATCH 25/25] build: move ycsb scripts to separate directory --- ycsb/report.sql | 32 ++++++++++++++++++++++++++++++++ {.ci => ycsb}/run-ycsb.sh | 0 {.ci => ycsb}/ycsb-setup.sh | 0 3 files changed, 32 insertions(+) create mode 100644 ycsb/report.sql rename {.ci => ycsb}/run-ycsb.sh (100%) rename {.ci => ycsb}/ycsb-setup.sh (100%) diff --git a/ycsb/report.sql b/ycsb/report.sql new file mode 100644 index 000000000..7e5903306 --- /dev/null +++ b/ycsb/report.sql @@ -0,0 +1,32 @@ +select r.threads, r.batch_size, r.operation_count, + max(n.throughput) as native_throughput, max(j17.throughput) as java17_throughput, max(j8.throughput) as java8_throughput, + case + when max(n.throughput) > max(j17.throughput) and max(n.throughput) > max(j8.throughput) then 'native' + when max(j17.throughput) > max(n.throughput) and max(j17.throughput) > max(j8.throughput) then 'java17' + when max(j8.throughput) > max(n.throughput) and max(j8.throughput) > max(j17.throughput) then 'java8' + else 'unknown' + end as highest_throughput, + min(n.read_avg) as native_read_avg, min(j17.read_avg) as java17, min(j8.read_avg) as java8, + case + when min(n.read_avg) < min(j17.read_avg) and min(n.read_avg) < min(j8.read_avg) then 'native' + when min(j17.read_avg) < min(n.read_avg) and min(j17.read_avg) < min(j8.read_avg) then 'java17' + when min(j8.read_avg) < min(n.read_avg) and min(j8.read_avg) < min(j17.read_avg) then 'java8' + else 'unknown' + end as lowest_read_avg, + min(n.read_p95) as native_read_p95, min(j17.read_p95) as java17, min(j8.read_p95) as java8, + case + when min(n.read_p95) < min(j17.read_p95) and min(n.read_p95) < min(j8.read_p95) then 'native' + when min(j17.read_p95) < min(n.read_p95) and min(j17.read_p95) < min(j8.read_p95) then 'java17' + when min(j8.read_p95) < min(n.read_p95) and min(j8.read_p95) < min(j17.read_p95) then 'java8' + else 'unknown' + end as lowest_read_p95 +from run r +inner join run n using (threads, batch_size, operation_count) +inner join run j8 using (threads, batch_size, operation_count) +inner join run j17 using (threads, batch_size, operation_count) +where n.deployment = 'native-uds' + and j8.deployment = 'jar-java8-uds' + and j17.deployment = 'jar-java17-uds' +group by r.threads, r.batch_size, r.operation_count +order by r.threads, r.batch_size, r.operation_count +; diff --git a/.ci/run-ycsb.sh b/ycsb/run-ycsb.sh similarity index 100% rename from .ci/run-ycsb.sh rename to ycsb/run-ycsb.sh diff --git a/.ci/ycsb-setup.sh b/ycsb/ycsb-setup.sh similarity index 100% rename from .ci/ycsb-setup.sh rename to ycsb/ycsb-setup.sh