David Van Couvering wrote:
I haven't heard any responses on this, so I am going to make the following changes/actions. Speak now or... speak later, I guess :)
- Use UUID to generate a unique id. On the engine side I will use the UUID service; on the client side I will use BasicUUID hardcoded.
To be clear, I am going to *copy* the BasicUUID class and make a duplicate in the org.apache.derby.client.am package.
- Remove toString() from PooledConnection and its subclasses in both client and engine code, as this class is never seen by application code
- Add a JIRA entry to allow for code-sharing between client and engine
- Add a JIRA entry to add a connection id to appropriate VTIs, where the id is the same as toString()
- Add a JIRA entry to add a new VTI that shows all active connections, where the id is the same as toString().
Thanks,
David
David Van Couvering wrote:
Daniel John Debrunner wrote:
David Van Couvering wrote:
Thanks for the quick reply, Dan, some responses below.
Daniel John Debrunner wrote:
I'd like to see some crisper definition around what the return from toString() is meant to represent for a connection.
- with this implementation, two different application connections from the same PooledConnection end up with the same toString() output, because the toString() of the underlying connection is used.
I am still learning about PooledConnection, not having dealt with this
before. I thought that PooledConnection was an interface only used by
app servers and other containers implementing connection pools. This
seems to match what I saw in my tests: regardless of what kind of access
mechanism I used (DriverManager, DataSource, ConnectionPoolDataSource or
XADataSource), I always ended up with an EmbedConnection30 or a
NetConnection being returned to the application, and never a
PooledConnection class.
You are right, I was talking about the connection's returned to the application, with your code in this case connection a and b have the same id, where pc is a PooledConnection object.
Connection a = pc.getConnection(); // some work Connection b = pc.getConnection();
Now I know a is closed once b is obtained, but if there share the same id then diagnostics might be confusing, as it will be impossible to tell the activity against connection a to that against connection b. Maybe that's ok, I'm not sure at the moment what you want the return from toString() to represent.
OK, now I understand. I think confusion is possible whatever approach we use. If we generate a new connection id for the underlying connection each time pc.getConnection() gets called, then it could be very confusing if the same physical connection had a problem and we couldn't determine it was the same physical connection in the trace logs.
At the same time, if two different apps get the same connection for two separate transactions, the person reading the log may get confused about what is happening. But I think this is actually manageable -- as long as you understand that connection pooling is going on, you should be able to trace a given transaction even if the connection is then placed back on the pool and used by someone else. I actually think the user might be more confused to see a new connection id for each transaction, as they might not understand that pooling is happening.
So my recommendation is to stick with the current implementation and let an EmbedConnection keep the same id for its lifetime. I also think I should just remove toString() from PooledConnection and its subclasses since its never seen by end users.
[snap]
I guess this could be solved if I follow Jack's original suggestion and
used hashCode(), since this will be unique across all objects in a VM.
But personally a simple integer id is a lot easier to read in a log file
than a longer hex number, or an even longer decimal number... The
connection id *could* be prepended with a system id, if such a thing
exists.
I thought we had decided that the hashCode was not guaranteed to be unique.
See my previous email, I had missed your email on this. So, I agree, hashCode is out.
That said, we need something that is unique across all classes *and* across classloaders and potentially even across VMs. The only alternative I can think of is UUID.
I've been thinking about the net client side as well. Even there, we can't assume that all ids will be generated from the same classloader. It is more than possible that the net client could be embedded in an app server or other middle-tier container. Since an app server normally has a different classloader for each application, we have to support an net client connection id that is unique across classloaders... So that should probably be a UUID as well...
That said, right now there is no way to share code such as UUIDFactory, UUID and BasicUUID. If I don't fix this, then I'll be copying a fairly large chunk of code across packages. It seems to me there are three ways I can go here:
(1) Copy BasicUUID and deal with the code duplication for now. Add a separate feature request to adjust our packaging and jars to allow for code shared across the client, tools and derby jar files (I'll call these "components" for lack of a better word).
(2) As part of this patch, create a new jar file called derbycommon.jar for packages/classes that span two or more components of Derby. Put BasicUUID in there.
(3) Same as (2), except that the whole services architecture goes into common, not just BasicUUID. I would prefer this from an architectural standpoint, but I'm a bit worried about memory bloat on the client side.
For sanity's sake, I would like to suggest (1). I think BasicUUID is stable enough that we'll be OK for the time being. But opinions here are much appreciated.
[snip]
Another thing that would be very nice is if, in client/server mode,
client connection toString() has a correlation to the resulting embedded
connection toString() created on the server side for that session. This
way you could get end-to-end correlation. I don't know if DRDA supports
this; if people think this is a good idea I can investigate...
I think that is there, there is a drda id which I think fulfills that purpose.
OK, I think this is also a separate feature request, do you agree?
[snip]
StatementCache is a VTI, you can execute the statement
select * from new org.apache.diag.StatementCache() AS SC
to get a list of statements in the cache.
And use a where clause to match the current statement to its cache entry by using the output from toString() of the PreparedStatement.
OK, I got it, thanks.
What I propose to do here is to add two feature requests:
- all VTIs that can be scoped by connection, such as StatementCache, LockTable, TransactionTable, etc., should add a new column for the owning connection id, which should match the output of toString().
- a diagnostics table for active connections,where the connection identifier in this table should match the output of toString().
Sound good?
Thanks,
David
