jackrabbit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Esteban Franqueiro (JIRA)" <j...@apache.org>
Subject [jira] Commented: (JCR-1388) Jackrabbit does not allow concurrent reads to the data store if copyWhenReading=false
Date Fri, 29 Feb 2008 16:24:51 GMT

    [ https://issues.apache.org/jira/browse/JCR-1388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12573815#action_12573815
] 

Esteban Franqueiro commented on JCR-1388:
-----------------------------------------

Hi.

> I have a few remarks, first about the source code 'style'. I use Eclipse and the Checkstyle
plugin, this should find most issues:
> - you should use spaces instead of tabs
> - return doesn't required brackets: return(false) should be changed to return false
> - catch (IOException e) {} should at least have a remark, but it's better to log the
exception (with stack trace)
> - you need to replace the file file headers 
> - use '} else {', '} finally {' and '} catch (Exception e) {' as in the Sun Java coding
guidelines
> - Don't declare all variables at the start of the method as in C. Declare them when /
just before using them
>  (for example, getResourceAsReader(), reader; there are others
> - Review the Javadocs rules (add comments, use the @param, @return tags)
> - Only use this. when required

Fixed all of those I think.

> Some other remarks:
> - I didn't see any test cases - please add one
> - Are prepareSchemaObjectPrefix and getResourceAsReader used somewhere? Don't add unused
methods

Removed them.

> - close() methods easting exceptions should be called closeSilently()

Renamed them.

> - You have removed the SQL statement remark, why? // SELECT ID, DATA FROM DATASTORE WHERE
ID = ?

Because I deleted the entire method.

> - If you are removing code, remove the lines, don't remark them (+//lastModified = ...)
> - getDatabaseResources, boolean success is always true
> - You hare remarked "lastModified = store.touch(getIdentifier(), lastModified)", why?

Fixed those.

> - Don't swallow exceptions (use IOException.initCause in DbInputStream.getStream())
> - Synchronization is very inconsistent (DbInputStream)

In what sense? The DbInputStream is not supposed to be used from multiple threads, so I only
synchronized the methods that are already synchronized in InputStream.

I'll attach the new patch and test.

Regards.

> Jackrabbit does not allow concurrent reads to the data store if copyWhenReading=false
> -------------------------------------------------------------------------------------
>
>                 Key: JCR-1388
>                 URL: https://issues.apache.org/jira/browse/JCR-1388
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-core
>    Affects Versions: 1.4
>         Environment: WinXP x64, Eclipse, remote SQL Server 2005
>            Reporter: Esteban Franqueiro
>         Attachments: JCR-1388-datastore-concurrent-reads.patch
>
>
> Jackrabbit does not allow concurrent reads to the data store if copyWhenReading=false,
even if maxConnections>1.
> See JCR-1184 for a test for this problem (run it with copyWhenReading=false).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message