JdbcDataSource synchronization
JdbcDataSource looks like it has a little funkiness going on. Why is
there a synchronize block there? Can multiple threads call getConnection
concurrently? If they can, this is not thread safe anyway. If they
can't, why is factory.call (or is it the close?) being protected with a
sync?
private Connection getConnection() throws Exception {
long currTime = System.currentTimeMillis();
if (currTime - connLastUsed > CONN_TIME_OUT) {
synchronized (this) {
Connection tmpConn = factory.call();
close();
connLastUsed = System.currentTimeMillis();
return conn = tmpConn;
}
} else {
connLastUsed = currTime;
return conn;
}
}
Re: JdbcDataSource synchronization
one JdbcDataSource has only one connection the connection object/the
connLastUsed etc needs to be protected.
On Tue, Nov 18, 2008 at 9:24 PM, Mark Miller <[EMAIL PROTECTED]> wrote:
> JdbcDataSource looks like it has a little funkiness going on. Why is there a
> synchronize block there? Can multiple threads call getConnection
> concurrently? If they can, this is not thread safe anyway. If they can't,
> why is factory.call (or is it the close?) being protected with a sync?
>
> private Connection getConnection() throws Exception {
> long currTime = System.currentTimeMillis();
> if (currTime - connLastUsed > CONN_TIME_OUT) {
> synchronized (this) {
> Connection tmpConn = factory.call();
> close();
> connLastUsed = System.currentTimeMillis();
> return conn = tmpConn;
> }
>
> } else {
> connLastUsed = currTime;
> return conn;
> }
> }
>
--
--Noble Paul
Re: JdbcDataSource synchronization
But if your going to sync a variable, you can't read/write it outside of
proper synchronization. Maybe I am just not following the code...
Noble Paul ??? ?? wrote:
one JdbcDataSource has only one connection the connection object/the
connLastUsed etc needs to be protected.
On Tue, Nov 18, 2008 at 9:24 PM, Mark Miller <[EMAIL PROTECTED]> wrote:
JdbcDataSource looks like it has a little funkiness going on. Why is there a
synchronize block there? Can multiple threads call getConnection
concurrently? If they can, this is not thread safe anyway. If they can't,
why is factory.call (or is it the close?) being protected with a sync?
private Connection getConnection() throws Exception {
long currTime = System.currentTimeMillis();
if (currTime - connLastUsed > CONN_TIME_OUT) {
synchronized (this) {
Connection tmpConn = factory.call();
close();
connLastUsed = System.currentTimeMillis();
return conn = tmpConn;
}
} else {
connLastUsed = currTime;
return conn;
}
}
Re: JdbcDataSource synchronization
it is not . conn is protected throughout
On Tue, Nov 18, 2008 at 9:48 PM, Mark Miller <[EMAIL PROTECTED]> wrote:
> But if your going to sync a variable, you can't read/write it outside of
> proper synchronization. Maybe I am just not following the code...
>
> Noble Paul ??? ?? wrote:
>>
>> one JdbcDataSource has only one connection the connection object/the
>> connLastUsed etc needs to be protected.
>>
>>
>>
>> On Tue, Nov 18, 2008 at 9:24 PM, Mark Miller <[EMAIL PROTECTED]>
>> wrote:
>>
>>>
>>> JdbcDataSource looks like it has a little funkiness going on. Why is
>>> there a
>>> synchronize block there? Can multiple threads call getConnection
>>> concurrently? If they can, this is not thread safe anyway. If they can't,
>>> why is factory.call (or is it the close?) being protected with a sync?
>>>
>>> private Connection getConnection() throws Exception {
>>> long currTime = System.currentTimeMillis();
>>> if (currTime - connLastUsed > CONN_TIME_OUT) {
>>>synchronized (this) {
>>> Connection tmpConn = factory.call();
>>> close();
>>> connLastUsed = System.currentTimeMillis();
>>> return conn = tmpConn;
>>>}
>>>
>>> } else {
>>>connLastUsed = currTime;
>>>return conn;
>>> }
>>> }
>>>
>>>
>>
>>
>>
>>
>
>
--
--Noble Paul
Re: JdbcDataSource synchronization
Okay. Lets agree to disagree on the connection protection.
How about conLastUsed? If multiple threads can enter that method (the
only reason you would need synchronization) then all of the threads will
access the same instance field 'conLastUsed' outside of synchronization,
possibly as both writes and reads. You can't share memory like that -
its unsafe.
Noble Paul നോബിള് नोब्ळ् wrote:
it is not . conn is protected throughout
On Tue, Nov 18, 2008 at 9:48 PM, Mark Miller <[EMAIL PROTECTED]> wrote:
But if your going to sync a variable, you can't read/write it outside of
proper synchronization. Maybe I am just not following the code...
Noble Paul ??? ?? wrote:
one JdbcDataSource has only one connection the connection object/the
connLastUsed etc needs to be protected.
On Tue, Nov 18, 2008 at 9:24 PM, Mark Miller <[EMAIL PROTECTED]>
wrote:
JdbcDataSource looks like it has a little funkiness going on. Why is
there a
synchronize block there? Can multiple threads call getConnection
concurrently? If they can, this is not thread safe anyway. If they can't,
why is factory.call (or is it the close?) being protected with a sync?
private Connection getConnection() throws Exception {
long currTime = System.currentTimeMillis();
if (currTime - connLastUsed > CONN_TIME_OUT) {
synchronized (this) {
Connection tmpConn = factory.call();
close();
connLastUsed = System.currentTimeMillis();
return conn = tmpConn;
}
} else {
connLastUsed = currTime;
return conn;
}
}
Re: JdbcDataSource synchronization
I took a very quick peek... looks like Mark is right, there are some
non-thread-safe practices going on there with both "conn" and
"connLastUsed". Prob also want another check inside the sync block so
multiple threads don't pile up at that sync block and all open a new
connection in turn (or just put the check inside the sync block).
Is a finalizer really needed? If so, hopefully the connection is
resistant to close() being called on it more than once?
-Yonik
On Tue, Nov 18, 2008 at 11:27 AM, Noble Paul നോബിള് नोब्ळ्
<[EMAIL PROTECTED]> wrote:
> it is not . conn is protected throughout
>
> On Tue, Nov 18, 2008 at 9:48 PM, Mark Miller <[EMAIL PROTECTED]> wrote:
>> But if your going to sync a variable, you can't read/write it outside of
>> proper synchronization. Maybe I am just not following the code...
>>
>> Noble Paul ??? ?? wrote:
>>>
>>> one JdbcDataSource has only one connection the connection object/the
>>> connLastUsed etc needs to be protected.
>>>
>>>
>>>
>>> On Tue, Nov 18, 2008 at 9:24 PM, Mark Miller <[EMAIL PROTECTED]>
>>> wrote:
>>>
JdbcDataSource looks like it has a little funkiness going on. Why is
there a
synchronize block there? Can multiple threads call getConnection
concurrently? If they can, this is not thread safe anyway. If they can't,
why is factory.call (or is it the close?) being protected with a sync?
private Connection getConnection() throws Exception {
long currTime = System.currentTimeMillis();
if (currTime - connLastUsed > CONN_TIME_OUT) {
synchronized (this) {
Connection tmpConn = factory.call();
close();
connLastUsed = System.currentTimeMillis();
return conn = tmpConn;
}
} else {
connLastUsed = currTime;
return conn;
}
}
>>>
>>>
>>>
>>>
>>
>>
>
>
>
> --
> --Noble Paul
>
Re: JdbcDataSource synchronization
Hmm, the import operation in DIH is single threaded. Each entity has a
separate instance of DataSource for itself. I think that we may not need the
synchronization at all.
On Tue, Nov 18, 2008 at 10:22 PM, Yonik Seeley <[EMAIL PROTECTED]> wrote:
> I took a very quick peek... looks like Mark is right, there are some
> non-thread-safe practices going on there with both "conn" and
> "connLastUsed". Prob also want another check inside the sync block so
> multiple threads don't pile up at that sync block and all open a new
> connection in turn (or just put the check inside the sync block).
>
> Is a finalizer really needed? If so, hopefully the connection is
> resistant to close() being called on it more than once?
>
> -Yonik
>
>
> On Tue, Nov 18, 2008 at 11:27 AM, Noble Paul നോബിള് नोब्ळ्
> <[EMAIL PROTECTED]> wrote:
> > it is not . conn is protected throughout
> >
> > On Tue, Nov 18, 2008 at 9:48 PM, Mark Miller <[EMAIL PROTECTED]>
> wrote:
> >> But if your going to sync a variable, you can't read/write it outside of
> >> proper synchronization. Maybe I am just not following the code...
> >>
> >> Noble Paul ??? ?? wrote:
> >>>
> >>> one JdbcDataSource has only one connection the connection object/the
> >>> connLastUsed etc needs to be protected.
> >>>
> >>>
> >>>
> >>> On Tue, Nov 18, 2008 at 9:24 PM, Mark Miller <[EMAIL PROTECTED]>
> >>> wrote:
> >>>
>
> JdbcDataSource looks like it has a little funkiness going on. Why is
> there a
> synchronize block there? Can multiple threads call getConnection
> concurrently? If they can, this is not thread safe anyway. If they
> can't,
> why is factory.call (or is it the close?) being protected with a sync?
>
> private Connection getConnection() throws Exception {
> long currTime = System.currentTimeMillis();
> if (currTime - connLastUsed > CONN_TIME_OUT) {
> synchronized (this) {
> Connection tmpConn = factory.call();
> close();
> connLastUsed = System.currentTimeMillis();
> return conn = tmpConn;
> }
>
> } else {
> connLastUsed = currTime;
> return conn;
> }
> }
>
>
> >>>
> >>>
> >>>
> >>>
> >>
> >>
> >
> >
> >
> > --
> > --Noble Paul
> >
>
--
Regards,
Shalin Shekhar Mangar.
Re: JdbcDataSource synchronization
As shalin says threadsafety is not given importance because DIH is
singlethreaded. But eventually we may move there .
But we indeed tried once w/ multithreaded indexing for around 3million
records and the performance gain we observed was <10% . We realized
that it was bound by Lucene's ability to index documents rather than
DIH's ability to produce it. So we dropped it in favor of simplicity
--Noble
On Tue, Nov 18, 2008 at 10:28 PM, Shalin Shekhar Mangar
<[EMAIL PROTECTED]> wrote:
> Hmm, the import operation in DIH is single threaded. Each entity has a
> separate instance of DataSource for itself. I think that we may not need the
> synchronization at all.
>
> On Tue, Nov 18, 2008 at 10:22 PM, Yonik Seeley <[EMAIL PROTECTED]> wrote:
>
>> I took a very quick peek... looks like Mark is right, there are some
>> non-thread-safe practices going on there with both "conn" and
>> "connLastUsed". Prob also want another check inside the sync block so
>> multiple threads don't pile up at that sync block and all open a new
>> connection in turn (or just put the check inside the sync block).
>>
>> Is a finalizer really needed? If so, hopefully the connection is
>> resistant to close() being called on it more than once?
>>
>> -Yonik
>>
>>
>> On Tue, Nov 18, 2008 at 11:27 AM, Noble Paul നോബിള് नोब्ळ्
>> <[EMAIL PROTECTED]> wrote:
>> > it is not . conn is protected throughout
>> >
>> > On Tue, Nov 18, 2008 at 9:48 PM, Mark Miller <[EMAIL PROTECTED]>
>> wrote:
>> >> But if your going to sync a variable, you can't read/write it outside of
>> >> proper synchronization. Maybe I am just not following the code...
>> >>
>> >> Noble Paul ??? ?? wrote:
>> >>>
>> >>> one JdbcDataSource has only one connection the connection object/the
>> >>> connLastUsed etc needs to be protected.
>> >>>
>> >>>
>> >>>
>> >>> On Tue, Nov 18, 2008 at 9:24 PM, Mark Miller <[EMAIL PROTECTED]>
>> >>> wrote:
>> >>>
>>
>> JdbcDataSource looks like it has a little funkiness going on. Why is
>> there a
>> synchronize block there? Can multiple threads call getConnection
>> concurrently? If they can, this is not thread safe anyway. If they
>> can't,
>> why is factory.call (or is it the close?) being protected with a sync?
>>
>> private Connection getConnection() throws Exception {
>> long currTime = System.currentTimeMillis();
>> if (currTime - connLastUsed > CONN_TIME_OUT) {
>> synchronized (this) {
>> Connection tmpConn = factory.call();
>> close();
>> connLastUsed = System.currentTimeMillis();
>> return conn = tmpConn;
>> }
>>
>> } else {
>> connLastUsed = currTime;
>> return conn;
>> }
>> }
>>
>>
>> >>>
>> >>>
>> >>>
>> >>>
>> >>
>> >>
>> >
>> >
>> >
>> > --
>> > --Noble Paul
>> >
>>
>
>
>
> --
> Regards,
> Shalin Shekhar Mangar.
>
--
--Noble Paul
