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

Specify isc_tpb_lock_timeout using JDBC connectionProperties property [JDBC407] #448

Closed
firebird-issue-importer opened this issue Sep 29, 2015 · 21 comments

Comments

@firebird-issue-importer

Submitted by: Vjacheslav Borisov (slavb18)

FBTpbMapper class could be improved to specify isc_tpb_lock_timeout param value, like

connectionProperties="TRANSACTION_READ_COMMITTED=isc_tpb_read_committed,isc_tpb_rec_version,isc_tpb_wait,isc_tpb_lock_timeout=5"

Commits: b73ea8f f2decd8 d5c8682 3814330 80ffa27

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Sep 29, 2015

Commented by: @mrotteveel

Thanks for the suggestion, but unfortunately I can't and won't do this. The current behavior has been the default since the start of Jaybid, changing this default could break applications that depend on the existing behavior. Also isc_tpb_lock_timeout is not available on older Firebird versions (1.0 and 1.5), so at least for 2.2.x this isn't an option at all as we promised to support those versions explicitly for Jaybird 2.2.x.

If you need different behavior, then I suggest you make a copy of isc_tpb_mapping.properties, add your own defaults and explicitly specify this file using the tpbMapping connection property.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Sep 29, 2015

Modified by: @mrotteveel

status: Open [ 1 ] => Closed [ 6 ]

resolution: Won't Fix [ 2 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Sep 30, 2015

Commented by: Vjacheslav Borisov (slavb18)

I don't understand, we do not change default behavior
default connection properties do no change, and this dont break applications
I want to specify transaction params in jdbc resource like

<Resource name="jdbc/base"

...
connectionProperties="TRANSACTION_READ_COMMITTED=isc_tpb_read_committed,isc_tpb_rec_version,isc_tpb_wait,isc_tpb_lock_timeout=5"
...
/>

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Sep 30, 2015

Commented by: Vjacheslav Borisov (slavb18)

i want to patch method FBTpbMapper.TransactionParameterBuffer

public static TransactionParameterBuffer processMapping(GDS gds, String mapping) throws FBResourceException {

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Sep 30, 2015

Commented by: @mrotteveel

My apologies, that is not how I read your request (especially as the FbTpbMapper is just an implementation detail). I have reopened the request. What you want is already somewhat possible if you use the Jaybird datasources directly, but as far as I know this isn't available for connection properties specified in the JDBC URL.

You have two options as a workaround:
1. Use the tpbMapping file as detailed in my previous comment (see also the Jaybird 2.1 Programmers manual)
2. If you use a Jaybird datasource, you might be able to specify the connection property nonStandardProperty=TRANSACTION_READ_COMMITTED=isc_tpb_read_committed,isc_tpb_rec_version,isc_tpb_wait,isc_tpb_lock_timeout=5

I am not entirely sure about option 2 as it contains multiple = signs, which might cause trouble in parsing.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Sep 30, 2015

Modified by: @mrotteveel

status: Closed [ 6 ] => Reopened [ 4 ]

resolution: Won't Fix [ 2 ] =>

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Sep 30, 2015

Commented by: Vjacheslav Borisov (slavb18)

alternatively we can use another value separator
connectionProperties="TRANSACTION_READ_COMMITTED=isc_tpb_read_committed,isc_tpb_rec_version,isc_tpb_wait,isc_tpb_lock_timeout*5"

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Sep 30, 2015

Commented by: Vjacheslav Borisov (slavb18)

Currently is not possible to specify isc_tpb_lock_timeout=5,

Internal Exception: java.sql.SQLException: Cannot create PoolableConnectionFactory (Resource Exception. Keyword isc_tpb_lock_timeout=5 unknown. Please check your mapping., error code: HY000)

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Sep 30, 2015

Commented by: Vjacheslav Borisov (slavb18)

method
public static TransactionParameterBuffer processMapping(GDS gds, String mapping) throws FBResourceException {

could be patched to look for '=' sign in token and split if it is exists, treating second part as integer and using

TransactionParameterBuffer.addArgument(int argumentType, int value);

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Sep 30, 2015

Commented by: Vjacheslav Borisov (slavb18)

created pull request on guthub

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Sep 30, 2015

Commented by: @mrotteveel

I see what you mean; I incorrectly assumed that processing a mapping like this would already work, and that it would be a matter of better exposing that in the connection properties.

I only see the pull request you made for JDBC408. Was this commented misplaced, or are you still working on it?

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Sep 30, 2015

Commented by: @mrotteveel

Never mind, I saw you added it to the same pull request as for JDBC407.

Would you mind splitting it into two separate pull requests?

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Oct 1, 2015

Commented by: Vjacheslav Borisov (slavb18)

created #8

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Oct 1, 2015

Commented by: Attila Molnár (e_pluribus_unum)

This works fine for me.

import org.firebirdsql.gds.TransactionParameterBuffer;

public void setConnection(Connection conn) {

FirebirdConnection fc = \(FirebirdConnection\) conn\.unwrap\(FirebirdConnection\.class\);
TransactionParameterBuffer tpb = fc\.createTransactionParameterBuffer\(\);
tpb\.addArgument\(TransactionParameterBuffer\.READ\);
tpb\.addArgument\(TransactionParameterBuffer\.READ\_COMMITTED\);
tpb\.addArgument\(TransactionParameterBuffer\.REC\_VERSION\);
if \(getLockTimeout\(\) == 0\) \{
	tpb\.addArgument\(TransactionParameterBuffer\.NOWAIT\);
\}
else \{
	tpb\.addArgument\(TransactionParameterBuffer\.WAIT\);
	tpb\.addArgument\(TransactionParameterBuffer\.LOCK\_TIMEOUT, getLockTimeout\(\)\);
\}
fc\.setTransactionParameters\(tpb\);

}

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Oct 1, 2015

Commented by: @mrotteveel

@attila That indeed works, but this issue is about specifying the transaction configuration in connection properties (or a tpbMapping file); the parser for that string format currently doesn't take tpb properties with values into account.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Oct 2, 2015

Commented by: @mrotteveel

Merged pull request #8 and added basic test. Also added change to master.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Nov 29, 2015

Commented by: @mrotteveel

This is already in 2.2.9 but wasn't documented as such in the release notes. Add note in 2.2.10 release notes.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Nov 29, 2015

Modified by: @mrotteveel

Fix Version: Jaybird 2.2.10 [ 10723 ]

Fix Version: Jaybird 3.0 [ 10440 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Nov 29, 2015

Modified by: @mrotteveel

status: Reopened [ 4 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jan 23, 2016

Modified by: @mrotteveel

Component: JDBC driver [ 10053 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 13, 2016

Modified by: @mrotteveel

status: Resolved [ 5 ] => Closed [ 6 ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants