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.

Sign up for GitHub

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

Jump to bottom

Driver unable to connect to CockroachDB #424

Closed
perezd opened this issue Aug 8, 2019 · 5 comments · Fixed by #425
Closed

Driver unable to connect to CockroachDB #424

perezd opened this issue Aug 8, 2019 · 5 comments · Fixed by #425
Labels
priority: low status: completed type: enhancement
Milestone
0.8.3

Comments

@perezd
Copy link

perezd commented Aug 8, 2019

Looks like something isn't compatible, either cockroachdb or this library.

Following this tutorial:
https://www.cockroachlabs.com/docs/v19.1/build-a-java-app-with-cockroachdb.html#insecure

DataSource snip (PGDataSource):

    pg.setServerName("localhost");
    pg.setPortNumber(26257);
    pg.setDatabaseName("bank");
    pg.setUser("maxroach");
    pg.setPassword(null);
    pg.setApplicationName("Testing");

Stack trace:

org.jdbi.v3.core.ConnectionException: com.impossibl.postgres.jdbc.PGSQLSimpleException: Connection Error: java.lang.NullPointerException
	at org.jdbi.v3.core.Jdbi.open(Jdbi.java:316)
	at org.jdbi.v3.core.Jdbi.withHandle(Jdbi.java:337)
        at io.grpc.stub.ServerCalls$UnaryServerCallHandler$UnaryServerCallListener.onHalfClose(ServerCalls.java:171)
	at io.grpc.internal.ServerCallImpl$ServerStreamListenerImpl.halfClosed(ServerCallImpl.java:331)
	at io.grpc.internal.ServerImpl$JumpToApplicationThreadServerStreamListener$1HalfClosed.runInContext(ServerImpl.java:808)
	at io.grpc.internal.ContextRunnable.run(ContextRunnable.java:37)
	at io.grpc.internal.SerializingExecutor.run(SerializingExecutor.java:123)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
Caused by: com.impossibl.postgres.jdbc.PGSQLSimpleException: Connection Error: java.lang.NullPointerException
	at com.impossibl.postgres.jdbc.ErrorUtils.makeSQLException(ErrorUtils.java:137)
	at com.impossibl.postgres.jdbc.ConnectionUtil.createConnection(ConnectionUtil.java:212)
	at com.impossibl.postgres.jdbc.AbstractDataSource.createConnection(AbstractDataSource.java:137)
	at com.impossibl.postgres.jdbc.PGDataSource.getConnection(PGDataSource.java:71)
	at com.impossibl.postgres.jdbc.PGDataSource.getConnection(PGDataSource.java:63)
	at org.jdbi.v3.core.Jdbi.open(Jdbi.java:301)
	... 13 more
Caused by: java.io.IOException: java.lang.NullPointerException
	at com.impossibl.postgres.protocol.v30.ServerConnectionFactory.translateConnectionException(ServerConnectionFactory.java:585)
	at com.impossibl.postgres.protocol.v30.ServerConnectionFactory.connect(ServerConnectionFactory.java:258)
	at com.impossibl.postgres.protocol.v30.ServerConnectionFactory.connect(ServerConnectionFactory.java:150)
	at com.impossibl.postgres.protocol.v30.ServerConnectionFactory.connect(ServerConnectionFactory.java:131)
	at com.impossibl.postgres.system.BasicContext.<init>(BasicContext.java:187)
	at com.impossibl.postgres.jdbc.PGDirectConnection.<init>(PGDirectConnection.java:221)
	at com.impossibl.postgres.jdbc.ConnectionUtil.createConnection(ConnectionUtil.java:203)
	... 17 more
Caused by: java.lang.NullPointerException
	at com.impossibl.postgres.protocol.v30.ServerConnectionFactory$3.handleComplete(ServerConnectionFactory.java:489)
	at com.impossibl.postgres.protocol.v30.StartupRequest$Handler.readyForQuery(StartupRequest.java:195)
	at com.impossibl.postgres.protocol.v30.MessageDispatchHandler.receiveReadyForQuery(MessageDispatchHandler.java:621)
	at com.impossibl.postgres.protocol.v30.MessageDispatchHandler.parseAndDispatch(MessageDispatchHandler.java:359)
	at com.impossibl.postgres.protocol.v30.MessageDispatchHandler.dispatch(MessageDispatchHandler.java:192)
	at com.impossibl.postgres.protocol.v30.MessageDispatchHandler.channelRead(MessageDispatchHandler.java:147)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:374)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:360)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:352)
	at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:321)
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:295)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:374)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:360)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:352)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1408)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:374)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:360)
	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:930)
	at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:163)
	at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:697)
	at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:632)
	at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:549)
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:511)
	at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:918)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	... 1 more
@rafiss
Copy link
Contributor

rafiss commented Aug 8, 2019

Thank you for testing with CockroachDB! This is occurring because pgjdbc-ng is assuming that BackendKeyData will always be provided. (Postgres docs)

Here's the offending method in ServerConnectionFactory.java. The NPE occurs when casting a null Integer to an int.

      @Override
      public void handleComplete(Integer processId, Integer secretKey, Map<String, String> parameterStatuses, List<Notice> notices) {
        startupParameterStatuses.putAll(parameterStatuses);
        startupKeyData.set(new KeyData(processId, secretKey));

        startupCompleted.countDown();
      }

CRDB does not currently support BackendKeyData. We took a stab at supporting it but it's still a work-in-progress and there is no immediate plan to complete it. If you're interested in following that, take a look at this PR: cockroachdb/cockroach#34520

In the meantime, I think a fix for this issue would be to not require BackendKeyData. Taking a look at pgjdbc, it seems like it defaults to a processId and secretKey of 0. I'm happy to work on the PR for that change in pgjdbc-ng.

@kdubb
Copy link
Member

kdubb commented Aug 8, 2019

If I remember correctly, BackendKeyData is only used for canceling long running requests. If we use zero's this should probably be detected in CancelRequestTask and skip the actual cancelation request with the appropriate logging.

rafiss added a commit to rafiss/pgjdbc-ng that referenced this issue Aug 8, 2019
@rafiss
Do not require BackendKeyData (impossibl#424)
c91433e 
Some databases, such as CockroachDB, do not send BackendKeyData on
connection initialization. Previously, the ServerConnectionFactory would
assume that BackendKeyData would always be present which caused a
NullPointerException. Now, the BackendKeyData defaults to 0 instead of
null.
@rafiss
Copy link
Contributor

rafiss commented Aug 8, 2019

Yeah, it is just for cancelling requests. (CockroachDB has a non-pg-compatible cancel query statement.)

I'm happy to add the check-and-logs, though without a check, I believe all that will happen is the cancel query would be rejected.

rafiss added a commit to rafiss/pgjdbc-ng that referenced this issue Aug 14, 2019
@rafiss
Do not require BackendKeyData (impossibl#424)
87c66c4 
Some databases, such as CockroachDB, do not send BackendKeyData on
connection initialization. Previously, the ServerConnectionFactory would
assume that BackendKeyData would always be present which caused a
NullPointerException. Now, the BackendKeyData defaults to 0 instead of
null.
rafiss added a commit to rafiss/pgjdbc-ng that referenced this issue Aug 14, 2019
@rafiss
Do not require BackendKeyData (impossibl#424)
94b2463 
Some databases, such as CockroachDB, do not send BackendKeyData on
connection initialization. Previously, the ServerConnectionFactory would
assume that BackendKeyData would always be present which caused a
NullPointerException. Now, the BackendKeyData defaults to 0 instead of
null.

If the BackendKeyData is not set, then the CancelRequestTask
short-circuits and logs a warning.
@rafiss rafiss mentioned this issue Aug 21, 2019
Merged
kdubb pushed a commit that referenced this issue Sep 11, 2019
@rafiss @kdubb
Do not require BackendKeyData (#424) (#425)
315e983 
Some databases, such as CockroachDB, do not send BackendKeyData on connection initialization. Previously, the ServerConnectionFactory would assume that BackendKeyData would always be present which caused a NullPointerException. Now, the BackendKeyData defaults to 0 instead of null.

If the BackendKeyData is not set, then the CancelRequestTask short-circuits and logs a warning.
@kdubb
Copy link
Member

kdubb commented Sep 11, 2019

@rafiss @perezd GitHub auto closed this during merge of #424.. Is it correct that it is currently fixed?

@rafiss
Copy link
Contributor

rafiss commented Sep 12, 2019

Yes, this issue is resolved with that change. I confirmed that my local repro case works now.

@kdubb kdubb added this to the 0.8.3 milestone Sep 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
priority: low status: completed type: enhancement
Projects
None yet
Milestone
0.8.3
Development

Successfully merging a pull request may close this issue.

Do not require BackendKeyData (#424) rafiss/pgjdbc-ng
3 participants
@perezd @kdubb @rafiss