Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: show hints at startup for psql connections #311

Draft
wants to merge 3 commits into
base: postgresql-dialect
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .ci/run-with-credentials.sh
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ e2e-psql)
echo "Starting PGAdapter"
ls -lh target
UBER_JAR="pgadapter.jar"
(java -jar target/"${UBER_JAR}" -p "${GOOGLE_CLOUD_PROJECT}" -i "${GOOGLE_CLOUD_INSTANCE}" -d "${GOOGLE_CLOUD_DATABASE_WITH_VERSION}" -e "${GOOGLE_CLOUD_ENDPOINT}" -s 4242 -ddl AutocommitImplicitTransaction > /dev/null 2>&1) &
(java -jar target/"${UBER_JAR}" -p "${GOOGLE_CLOUD_PROJECT}" -i "${GOOGLE_CLOUD_INSTANCE}" -d "${GOOGLE_CLOUD_DATABASE_WITH_VERSION}" -e "${GOOGLE_CLOUD_ENDPOINT}" -s 4242 -ddl AutocommitImplicitTransaction -disable_psql_hints > /dev/null 2>&1) &
BACK_PID=$!
sleep 1
# execute psql and evaluate result
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import com.google.cloud.spanner.pgadapter.wireoutput.TerminateResponse;
import com.google.cloud.spanner.pgadapter.wireprotocol.BootstrapMessage;
import com.google.cloud.spanner.pgadapter.wireprotocol.WireMessage;
import com.google.common.annotations.VisibleForTesting;
import com.google.spanner.admin.database.v1.InstanceName;
import com.google.spanner.v1.DatabaseName;
import java.io.DataOutputStream;
Expand Down Expand Up @@ -81,6 +82,7 @@ public class ConnectionHandler extends Thread {

private final ProxyServer server;
private final Socket socket;
private final boolean isTcpSocket;
private final Map<String, IntermediatePreparedStatement> statementsMap = new HashMap<>();
private final Map<String, IntermediatePortalStatement> portalsMap = new HashMap<>();
private static final Map<Integer, IntermediateStatement> activeStatementsMap =
Expand All @@ -100,10 +102,16 @@ public class ConnectionHandler extends Thread {
private WellKnownClient wellKnownClient;
private ExtendedQueryProtocolHandler extendedQueryProtocolHandler;

@VisibleForTesting
ConnectionHandler(ProxyServer server, Socket socket) {
this(server, socket, true);
}

ConnectionHandler(ProxyServer server, Socket socket, boolean isTcpSocket) {
super("ConnectionHandler-" + CONNECTION_HANDLER_ID_GENERATOR.incrementAndGet());
this.server = server;
this.socket = socket;
this.isTcpSocket = isTcpSocket;
this.secret = new SecureRandom().nextInt();
setDaemon(true);
logger.log(
Expand Down Expand Up @@ -555,6 +563,11 @@ public void setWellKnownClient(WellKnownClient wellKnownClient) {
this.wellKnownClient = wellKnownClient;
}

/** Returns true if this connection uses a TCP connection. */
public boolean isTcpConnection() {
return this.isTcpSocket;
}

/** Status of a {@link ConnectionHandler} */
public enum ConnectionStatus {
UNAUTHENTICATED,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ void runTcpServer(CountDownLatch startupLatch, CountDownLatch stoppedLatch) thro
this.serverSockets.add(tcpSocket);
this.localPort = tcpSocket.getLocalPort();
tcpStartedLatch.countDown();
runServer(tcpSocket, startupLatch, stoppedLatch);
runServer(tcpSocket, startupLatch, stoppedLatch, true);
}

void runDomainSocketServer(CountDownLatch startupLatch, CountDownLatch stoppedLatch)
Expand All @@ -222,7 +222,7 @@ void runDomainSocketServer(CountDownLatch startupLatch, CountDownLatch stoppedLa
AFUNIXServerSocket domainSocket = AFUNIXServerSocket.newInstance();
domainSocket.bind(AFUNIXSocketAddress.of(tempDir), this.options.getMaxBacklog());
this.serverSockets.add(domainSocket);
runServer(domainSocket, startupLatch, stoppedLatch);
runServer(domainSocket, startupLatch, stoppedLatch, false);
} catch (SocketException socketException) {
logger.log(
Level.SEVERE,
Expand All @@ -235,13 +235,16 @@ void runDomainSocketServer(CountDownLatch startupLatch, CountDownLatch stoppedLa
}

void runServer(
ServerSocket serverSocket, CountDownLatch startupLatch, CountDownLatch stoppedLatch)
ServerSocket serverSocket,
CountDownLatch startupLatch,
CountDownLatch stoppedLatch,
boolean isTcpSocket)
throws IOException {
startupLatch.countDown();
awaitRunning();
try {
while (isRunning()) {
createConnectionHandler(serverSocket.accept());
createConnectionHandler(serverSocket.accept(), isTcpSocket);
}
} catch (SocketException e) {
// This is a normal exception, as this will occur when Server#stopServer() is called.
Expand All @@ -264,8 +267,8 @@ void runServer(
* @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(Socket socket, boolean isTcpSocket) {
ConnectionHandler handler = new ConnectionHandler(this, socket, isTcpSocket);
register(handler);
handler.start();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public enum DdlTransactionMode {
private static final String OPTION_BINARY_FORMAT = "b";
private static final String OPTION_AUTHENTICATE = "a";
private static final String OPTION_DISABLE_AUTO_DETECT_CLIENT = "disable_auto_detect_client";
private static final String OPTION_DISABLE_PSQL_HINTS = "disable_psql_hints";
private static final String OPTION_DISABLE_DEFAULT_LOCAL_STATEMENTS =
"disable_default_local_statements";
private static final String OPTION_PSQL_MODE = "q";
Expand Down Expand Up @@ -109,6 +110,7 @@ public enum DdlTransactionMode {
private final boolean binaryFormat;
private final boolean authenticate;
private final boolean disableAutoDetectClient;
private final boolean disablePsqlHints;
private final boolean disableDefaultLocalStatements;
private final boolean requiresMatcher;
private final DdlTransactionMode ddlTransactionMode;
Expand Down Expand Up @@ -160,6 +162,7 @@ public OptionsMetadata(String[] args) {
this.binaryFormat = commandLine.hasOption(OPTION_BINARY_FORMAT);
this.authenticate = commandLine.hasOption(OPTION_AUTHENTICATE);
this.disableAutoDetectClient = commandLine.hasOption(OPTION_DISABLE_AUTO_DETECT_CLIENT);
this.disablePsqlHints = commandLine.hasOption(OPTION_DISABLE_PSQL_HINTS);
this.disableDefaultLocalStatements =
commandLine.hasOption(OPTION_DISABLE_DEFAULT_LOCAL_STATEMENTS);
this.requiresMatcher =
Expand Down Expand Up @@ -216,6 +219,7 @@ public OptionsMetadata(
this.textFormat = textFormat;
this.binaryFormat = forceBinary;
this.authenticate = authenticate;
this.disablePsqlHints = false;
this.disableAutoDetectClient = false;
this.disableDefaultLocalStatements = false;
this.requiresMatcher = requiresMatcher;
Expand Down Expand Up @@ -273,8 +277,7 @@ private int buildProxyPort(CommandLine commandLine) {

private String buildSocketFile(CommandLine commandLine) {
// Unix domain sockets are disabled by default on Windows.
String directory =
commandLine.getOptionValue(OPTION_SOCKET_DIR, isWindows() ? "" : DEFAULT_SOCKET_DIR).trim();
String directory = getSocketDir();
if (!Strings.isNullOrEmpty(directory)) {
if (directory.charAt(directory.length() - 1) != File.separatorChar) {
directory += File.separatorChar;
Expand All @@ -284,6 +287,12 @@ private String buildSocketFile(CommandLine commandLine) {
return "";
}

public String getSocketDir() {
return commandLine
.getOptionValue(OPTION_SOCKET_DIR, isWindows() ? "" : DEFAULT_SOCKET_DIR)
.trim();
}

private int buildMaxBacklog(CommandLine commandLine) {
int backlog =
Integer.parseInt(
Expand Down Expand Up @@ -508,6 +517,11 @@ private CommandLine buildOptions(String[] args) {
"This option turns off automatic detection of well-known clients. "
+ "Use this option if you do not want PGAdapter to automatically apply query "
+ "replacements based on the client that is connected to PGAdapter.");
options.addOption(
null,
OPTION_DISABLE_PSQL_HINTS,
false,
"This option turns off the automatic hints that are shown for new psql connections.");
options.addOption(
null,
OPTION_DISABLE_DEFAULT_LOCAL_STATEMENTS,
Expand Down Expand Up @@ -751,6 +765,10 @@ public boolean shouldAutoDetectClient() {
return !this.disableAutoDetectClient;
}

public boolean showHints() {
return !this.disablePsqlHints;
}

public boolean useDefaultLocalStatements() {
return !this.disableDefaultLocalStatements;
}
Expand All @@ -759,6 +777,10 @@ public boolean requiresMatcher() {
return this.requiresMatcher;
}

public boolean hasPsqlCommandLineOptions() {
return this.commandLine.hasOption(OPTION_PSQL_MODE);
}

public boolean isReplaceJdbcMetadataQueries() {
return this.replaceJdbcMetadataQueries;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
import com.google.cloud.spanner.pgadapter.statements.local.SelectCurrentCatalogStatement;
import com.google.cloud.spanner.pgadapter.statements.local.SelectCurrentDatabaseStatement;
import com.google.cloud.spanner.pgadapter.statements.local.SelectCurrentSchemaStatement;
import com.google.cloud.spanner.pgadapter.utils.HintGenerator.Hint;
import com.google.cloud.spanner.pgadapter.wireoutput.NoticeResponse;
import com.google.cloud.spanner.pgadapter.wireoutput.NoticeResponse.NoticeSeverity;
import com.google.common.collect.ImmutableList;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -59,6 +62,24 @@ public ImmutableList<LocalStatement> getLocalStatements(ConnectionHandler connec
}
return ImmutableList.of(new ListDatabasesStatement(connectionHandler));
}

@Override
public Iterable<NoticeResponse> createStartupNoticeResponses(ConnectionHandler connection) {
if (connection.getServer().getOptions().hasPsqlCommandLineOptions()) {
return ImmutableList.of(
new NoticeResponse(
connection.getConnectionMetadata().peekOutputStream(),
NoticeSeverity.INFO,
"The 'psql mode' (-q) command line argument for PGAdapter is deprecated.",
"PGAdapter automatically recognizes connections from psql.\n PGAdapter can safely be started without the -q argument."));
}
Hint hint = HintGenerator.getRandomHint(connection);
if (hint != null) {
return ImmutableList.of(
new NoticeResponse(connection.getConnectionMetadata().peekOutputStream(), hint));
}
return super.createStartupNoticeResponses(connection);
}
},
JDBC {
@Override
Expand Down Expand Up @@ -118,6 +139,11 @@ public ImmutableList<LocalStatement> getLocalStatements(ConnectionHandler connec
}
return EMPTY_LOCAL_STATEMENTS;
}

/** Creates specific notice messages for a client after startup. */
public Iterable<NoticeResponse> createStartupNoticeResponses(ConnectionHandler connection) {
return ImmutableList.of();
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
// 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.utils;

import com.google.api.core.InternalApi;
import com.google.cloud.spanner.pgadapter.ConnectionHandler;
import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata;
import com.google.common.collect.ImmutableList;
import java.util.Random;
import javax.annotation.Nullable;

/**
* Generates tips for PGAdapter usage and options. This is used to show hints for new features and
* general tips for psql connections. Hints are not generated for other (non-text) clients.
*/
@InternalApi
public class HintGenerator {
public static class Hint {
private final String message;
private final String hint;

private Hint(String message, String hint) {
// Add an extra space to align the 'TIP' with the 'HINT'.
this.message = " " + message;
this.hint = hint;
}

public String getMessage() {
return message;
}

public String getHint() {
return hint;
}
}

private HintGenerator() {}

/** Generates a random hint for the given connection. */
public static @Nullable Hint getRandomHint(ConnectionHandler connection) {
OptionsMetadata options = connection.getServer().getOptions();
if (!options.showHints()) {
return null;
}

ImmutableList.Builder<Hint> builder = ImmutableList.builder();
// Authentication hints.
if (!options.shouldAuthenticate()) {
builder.add(
new Hint(
"Start PGAdapter with -a to enable authentication.",
"See https://github.com/GoogleCloudPlatform/pgadapter/blob/postgresql-dialect/docs/authentication.md"));
}

// Connection hints.
if (options.hasDefaultConnectionUrl()) {
builder.add(
new Hint(
"Start PGAdapter without the -d option to enable the use of the \\c meta-command.",
"See https://github.com/GoogleCloudPlatform/pgadapter/blob/postgresql-dialect/docs/connection_options.md"));
} else {
builder.add(
new Hint(
"You can connect to any Cloud Spanner database using a fully qualified database name.",
"Example: 'psql -d \"projects/my-project/instances/my-instance/databases/my-database\".\n"
+ "See https://github.com/GoogleCloudPlatform/pgadapter/blob/postgresql-dialect/docs/connection_options.md"));
}
if (options.isDomainSocketEnabled() && connection.isTcpConnection()) {
builder.add(
new Hint(
"PGAdapter supports Unix domain sockets.",
String.format(
"Use '-h %s' instead of '-h localhost' to connect using Unix domain sockets.",
options.getSocketDir())));
}

// COPY hints.
builder.add(
new Hint(
"Use COPY my_table FROM STDIN [BINARY] to bulk insert data into a table from a local file or a PostgreSQL database.",
"See https://github.com/GoogleCloudPlatform/pgadapter/blob/postgresql-dialect/docs/copy.md"));
builder.add(
new Hint(
"COPY my_table FROM STDIN supports non-atomic operations to copy more than 20,000 mutations.",
"Execute 'SET SPANNER.AUTOCOMMIT_DML_MODE='PARTITIONED_NON_ATOMIC;' before executing the COPY command.\n"
+ "See https://github.com/GoogleCloudPlatform/pgadapter/blob/postgresql-dialect/docs/copy.md#non-atomic-copy-from-stdin-example"));
builder.add(
new Hint(
"Use COPY my_table TO STDOUT [BINARY] to copy all data from a Cloud Spanner table to a PostgreSQL database or to a local file.",
"See https://github.com/GoogleCloudPlatform/pgadapter/blob/postgresql-dialect/docs/copy.md"));

// DDL hints.
builder.add(
new Hint(
"Use the PGAdapter '-ddl' command line argument to emulate support for DDL transactions.",
"See https://github.com/GoogleCloudPlatform/pgadapter/blob/postgresql-dialect/docs/ddl.md"));
ImmutableList<Hint> possibleHints = builder.build();
if (possibleHints.isEmpty()) {
return null;
}
return possibleHints.get(new Random().nextInt(possibleHints.size()));
}
}
Loading