connection toString should uniquely identify the connection

• Key: DERBY-243
• Type: Improvement
• Status: Closed
• Resolution: Fixed
• Priority: Trivial
• Assignee: David Van Couvering
• Reporter: Kathey Marsden
• Component/s: JDBC
• Affects Version/s: 10.0.2.0, 10.0.2.1, 10.0.2.2, 10.1.1.0
• Fix Version/s: 10.1.1.0

Description

The toString() on the Derby connection doesn't print
unique information.
for example System.out.println(conn) prints:
EmbedConnection in the case of derby embedded

It would be great if the toString() method for connections could be used to differentiate one connection from another.

Comments

[Kathey Marsden]

I am not sure if XA Connections and Pooled Connections have the same issue. I didn't immediately see an override of the toString() method in BrokeredConnection.java like there is for EmbedConnection

[David Van Couvering]

I am attaching the patch that should resolve this bug. I added toString() to the
appropriate connection classes and updated the dataSourcePermissions.java and
dataSourcePermissions_net.java tests to test toString(). I mostly just verify
that two connections do not have the same string and follow the expected
pattern. I had to "sed" out the actual connection ids in the output so we
could guarantee a pass even if the ids change.

Here is the output of svn status

M java\engine\org\apache\derby\impl\jdbc\EmbedConnection.java
M java\engine\org\apache\derby\jdbc\EmbedPooledConnection.java
M java\engine\org\apache\derby\iapi\jdbc\BrokeredConnection.java
M java\testing\org\apache\derbyTesting\functionTests\tests\jdbcapi\dataSour
cePermissions.java
A java\testing\org\apache\derbyTesting\functionTests\tests\jdbcapi\dataSour
cePermissions_sed.properties
M java\testing\org\apache\derbyTesting\functionTests\tests\jdbcapi\copyfile
s.ant
M java\testing\org\apache\derbyTesting\functionTests\tests\derbynet\dataSou
rcePermissions_net_sed.properties
M java\testing\org\apache\derbyTesting\functionTests\tests\derbynet\dataSou
rcePermissions_net.java
M java\testing\org\apache\derbyTesting\functionTests\master\DerbyNet\dataSo
urcePermissions_net.out
M java\testing\org\apache\derbyTesting\functionTests\master\dataSourcePermi
ssions.out
M java\testing\org\apache\derbyTesting\functionTests\master\DerbyNetClient\
dataSourcePermissions_net.out
M java\client\org\apache\derby\client\ClientPooledConnection.java
M java\client\org\apache\derby\client\am\Connection.java
M java\client\org\apache\derby\client\am\LogicalConnection.java

David

[David Van Couvering]

I have replaced the old patch with this one. This one uses UUID to identify a connection string.
It also provides independent connection strings for PooledConnection classes.

I also added some new tests to jdbcapi/checkDataSource and jdbcapi/CheckDataSource30. I refactored
the code to ensure two toStrings() are not unique for two separate objects and put this
into TestUtil.

All tests pass, except for tools/dblook_test.java, which has a single-line diff that looks completely
unrelated. I am investigating and may report a separate bug on this.

Here is svn status output:

M java/engine/org/apache/derby/impl/jdbc/EmbedConnection.java
M java/engine/org/apache/derby/iapi/jdbc/BrokeredConnection.java
M java/engine/org/apache/derby/jdbc/EmbedPooledConnection.java
M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/dataSour
cePermissions.java
M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/checkDat
aSource.java
M java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/dataSou
rcePermissions_net_sed.properties
M java/testing/org/apache/derbyTesting/functionTests/master/DerbyNet/dataSo
urcePermissions_net.out
M java/testing/org/apache/derbyTesting/functionTests/master/dataSourcePermi
ssions.out
M java/testing/org/apache/derbyTesting/functionTests/master/DerbyNetClient/
dataSourcePermissions_net.out
M java/testing/org/apache/derbyTesting/functionTests/master/checkDataSource
.out
M java/testing/org/apache/derbyTesting/functionTests/master/checkDataSource
30.out
M java/testing/org/apache/derbyTesting/functionTests/suites/derbyall.proper
ties
M java/testing/org/apache/derbyTesting/functionTests/util/TestUtil.java
M java/client/org/apache/derby/client/ClientPooledConnection.java
M java/client/org/apache/derby/client/am/Connection.java
A java/client/org/apache/derby/client/am/BasicUUID.java
A java/client/org/apache/derby/client/am/BasicUUIDFactory.java
M java/client/org/apache/derby/client/am/LogicalConnection.java

[David Van Couvering]

This feature can be implemented but will not provide a unique string value until DERBY-243 is resolved and we have a unique connection string

[Kathey Marsden]

I performed only a very rough review of this diff. I am hoping someone else can do a more detailed review once the following issues have been resolved.

1) I tend to think that it is confusing to have a separate UUID for the client connection. In a client server environment, each connection has two separate UUID's, which could get confusing. Especially since the connection id semantics change when dealing with client. There is an existing Derby entry Derby-293 to correlate the client and server connections. Might it be good to defer the client part of this work until that is considered?

2) I think it would be good to document the connection id semantics in the Jira entry and the code.

[David Van Couvering]

In response to Kathey's request, I am providing information on the semantics for providing a unique string representation for a connection.

First, it is important to identify three major types of connections in Derby:

POOLED CONNECTIONS
Pooled Connections are a subtype of javax.sql.PooledConnection. They represent a connection stored on a pool. References to PooledConnections are never directly visible to application code. They are used by implementors of connection pools so that management of pooled connections can be standardized across JDBC driver providers.

PROXY CONNECTIONS
Proxy connections are a class of connections that, although they implement the java.sql.Connection interface, don't directly implement the methods of java.sql.Connection. Instead, they act as a proxy to an underlying physical connection, and allow containers to "inject" container logic when key connectoin lifecycle events occur. These are also sometimes called "logical connections" or "brokered connections".

PHYSICAL CONNECTIONS

These are the connection classes that do the "heavy lifting" of actually implementing the methods on java.sql.Connection. They are the underlying physical connections wrapped by proxy connections.

CONNECTION STRING SEMANTICS
With these definitions in place, here are the semantics of connection strings:

- When a physical connection is created, it is assigned a unique UUID that is unchanged for the lifetime of the connection.

- When an application calls Connection.toString(), it gets the string "Connection_<UUID>" of the underlying physical connection, regardless of where the application has a reference to the physical connection itself a reference to a proxy connection (aka brokered connection) that wraps the physical connection.

- A pooled connection is assigned a separate UUID from a physical connection. When a container calls PooledConnection.toString(), it gets the string "PooledConnection_<UUID>". This is useful for developers implementing connection pools when they are trying to debug pooled connections

[David Van Couvering]

I have attached the latest patch in response to Kathey's comments.

I added comments to the Javadoc for the connection string semantics, and I removed
the toString() code for the client side. derbyall passes with the exception of
parameterMapping and backupRestore, which others have noted are currently failing.

Output of svn status:

M java/engine/org/apache/derby/impl/jdbc/EmbedConnection.java
M java/engine/org/apache/derby/iapi/jdbc/BrokeredConnection.java
M java/engine/org/apache/derby/jdbc/EmbedPooledConnection.java
M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/dataSourcePermissions.java
M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/checkDataSource.java
M java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/dataSourcePermissions_net_sed.properties
M java/testing/org/apache/derbyTesting/functionTests/master/DerbyNet/dataSourcePermissions_net.out
M java/testing/org/apache/derbyTesting/functionTests/master/dataSourcePermissions.out
M java/testing/org/apache/derbyTesting/functionTests/master/DerbyNetClient/dataSourcePermissions_net.out
M java/testing/org/apache/derbyTesting/functionTests/master/checkDataSource.out
M java/testing/org/apache/derbyTesting/functionTests/master/checkDataSource30.out
M java/testing/org/apache/derbyTesting/functionTests/util/TestUtil.java

[David Van Couvering]

In my investigations to understand how to add the connection string to VTIs, I have thought further about this item, and it has become clear to me that storing the unique id in the Connection object is putting it at the wrong level. If, for example, we ever want to provide a separate API for obtaining a session into the Derby database outside of JDBC, this information would not be available.

For this reason, it makes much more sense to use the LanguageConnectionContext as the owner of a unique identification for the session.

As some of our discussion has revealed, the LanguageConnectionContext already has an instance number, printed out as either "SESSIONID" or "LCCID" in the logs and VTIs. One could argue that we should just use this id. However, this id is a simple static integer incremented each time a new LanguageConnectionContext is created, and my concerns about loss of uniqueness across classloaders stands for this id, and I think it is going to cause us problems, particularly in the context of an app server where there is normally a separate classloader for each deployed application.

I am recommending that we add a new method to LanguageConnectionContext, UUID getId(). Connection.toString() will return "Connection_" + the String representation of this id.

Also, today our error logs and VTIs today all use the existing instance number as the LanguageConnectionContext id (SESSIONID or LCCID). I would like to propose that this be replaced to print out the new UUID instead.

Pooled connections, as we have already discussed, have their own separate identity, and I am recommending that we keep them using their own UUID as it is already implemented.

[Daniel John Debrunner]

David wrote
> For this reason, it makes much more sense to use the LanguageConnectionContext as the owner of a unique identification for the session.

I agree, the LCC is the "physical connection", thus having it define the unique identification matches the definition.

[David Van Couvering]

Here is an updated patch for this item. This addresses my previous comment by pushing
the unique id down to the LanguageConnectionContext.

I also modified the trace and log output to use this id rather than the LCC instance
number. This means that you can correlate the output from Connection.toString()
with the output in the error log and traces.

Finally, upon further rumination, I decided to remove the "Connection_" and
"PooledConnection_" prefix from the toString() output. As I worked with this
code, it seemed odd to have ths prefix here, when the user will normally print out
debug code like

  System.out.println("Connection: " + conn.toString());

Removing the "Connection_" prefix also means the output of toString()
exactly matches the output of lcc.getId(), which may make it easier
in the future to do filters against VTIs using the output of conn.toString().

Output of svn status:

M java/engine/org/apache/derby/impl/sql/GenericStatement.java
M java/engine/org/apache/derby/impl/sql/conn/GenericLanguageConnectionContext.java
M java/engine/org/apache/derby/impl/sql/conn/GenericLanguageConnectionFactory.java
M java/engine/org/apache/derby/impl/sql/execute/BasicNoPutResultSetImpl.java
M java/engine/org/apache/derby/impl/sql/execute/NoRowsResultSetImpl.java
M java/engine/org/apache/derby/impl/sql/GenericPreparedStatement.java
M java/engine/org/apache/derby/impl/jdbc/EmbedConnection.java
M java/engine/org/apache/derby/iapi/sql/conn/LanguageConnectionContext.java
M java/engine/org/apache/derby/iapi/jdbc/BrokeredConnection.java
M java/engine/org/apache/derby/jdbc/EmbedPooledConnection.java
M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/dataSourcePermissions.java
M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/checkDataSource.java
M java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/dataSourcePermissions_net_sed.properties
M java/testing/org/apache/derbyTesting/functionTests/master/DerbyNet/dataSourcePermissions_net.out
M java/testing/org/apache/derbyTesting/functionTests/master/dataSourcePermissions.out
M java/testing/org/apache/derbyTesting/functionTests/master/DerbyNetClient/dataSourcePermissions_net.out
M java/testing/org/apache/derbyTesting/functionTests/master/checkDataSource.out
M java/testing/org/apache/derbyTesting/functionTests/master/checkDataSource30.out
M java/testing/org/apache/derbyTesting/functionTests/suites/derbyall.properties
M java/testing/org/apache/derbyTesting/functionTests/util/TestUtil.java


David

[David Van Couvering]

A rescan of svn status revealed that the patch included my modification of
derby.properties to use the "test: durability mode. Oops!

This updated patch does not include this modification.

[David Van Couvering]

Good catch by Kathey. I added some SQL to lang/miscerrors.sql to print out all the rows
from the error log using the ErrorLogReader VTI, and it printed out the new ID fine,
except that it was truncated because the column length for the id was 15 when it needs
to be 36 for a UUID.

I am attaching an updated patch that resolves this issue.

I modified the ErrorLogReader VTI to set the length to 36, and the data printed out
fine. I also include the change to miscerrors.sql so that we are excercising this
VTI in our regression tests.

Thanks, Kathey!

David

[David Van Couvering]

Updated patch for this issue. Here is a summary of what I changed. A lot of what I did was remove code I had put in,
so I think you'll see a much simpler patch.

- Reverted back to using a simple integer for the GenericLanguageConnectionContext id
- All trace and log statements and the ErrorLogReader VTI go back to using the simple integer SESSIONID/LCCID
- EmbedConnection uses the LCC ID as its toString value
- BrokeredConnection continues to use the underlying physical connection's toString value for it's toString value
- EmbedPooledConnection now uses a simple integer instead of a UUID
- Reworked the tests to open 10 connections and make sure they are all unique. It does this for a DriverManager
  connection, a connection from a DataSource, a PooledConnection from a ConnectionPoolDataSource and an
  XAConnection from an XADataSource. It also ensures connections obtained from the same PooledConnection
  have the same toString() value
- I no longer print out the toString() value in the test.
- I modified lang/miscerrors.sql to select from the ErrorLogReader VTI so we can validate that this output is
  correct.

Ran derbyall, everything passed except for a strange error in encryptionAll/multi/stress which appears completely
unrelated. Every now and then my stress tests fail for reasons I don't fully fathom. Should I be investigating these
or is this a known issue?

Here is the output of svn status:

M java/engine/org/apache/derby/impl/jdbc/EmbedConnection.java
M java/engine/org/apache/derby/iapi/jdbc/BrokeredConnection.java
M java/engine/org/apache/derby/jdbc/EmbedPooledConnection.java
M java/testing/org/apache/derbyTesting/functionTests/tests/lang/miscerrors.sql
M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/checkDataSource.java
M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/checkDataSource30.java
M java/testing/org/apache/derbyTesting/functionTests/master/miscerrors.out

[David Van Couvering]

Revised patch based on Oystein's most recent review.

Changes since previous patch:
- Cleared up extraneous spaces
- Used Integer.toString(int) rather than new Integer(int).toString()
- Restored original checkDataSource30.java, it is no longer part of this patch
- Fixed Javadoc for EmbedPooledConnection.toString()

derbyall passes with no errors

svn status:
M java/engine/org/apache/derby/impl/jdbc/EmbedConnection.java
M java/engine/org/apache/derby/iapi/jdbc/BrokeredConnection.java
M java/engine/org/apache/derby/jdbc/EmbedPooledConnection.java
M java/testing/org/apache/derbyTesting/functionTests/tests/lang/miscerrors.sql
M java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/checkDataSource.java
M java/testing/org/apache/derbyTesting/functionTests/master/miscerrors.out

[Kathey Marsden]

Committed patch to print connection id in connection.toString

Sending java\engine\org\apache\derby\iapi\jdbc\BrokeredConnection.java
Sending java\engine\org\apache\derby\impl\jdbc\EmbedConnection.java
Sending java\engine\org\apache\derby\jdbc\EmbedPooledConnection.java
Sending java\testing\org\apache\derbyTesting\functionTests\master\miscerrors.out
Sending java\testing\org\apache\derbyTesting\functionTests\tests\jdbcapi\checkDataSource.java
Sending java\testing\org\apache\derbyTesting\functionTests\tests\lang\miscerrors.sql
Transmitting file data ......
Committed revision 189704.
$

[David Van Couvering]

Checked in by Kathey

[Kathey Marsden]

fixed