Mailing-List: contact dev-help@jackrabbit.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@jackrabbit.apache.org
Message-ID: <999035007.1204302291376.JavaMail.jira@brutus>
Date: Fri, 29 Feb 2008 08:24:51 -0800 (PST)
From: "Esteban Franqueiro (JIRA)" <jira@apache.org>
To: dev@jackrabbit.apache.org
Subject: [jira] Commented: (JCR-1388) Jackrabbit does not allow concurrent
 reads to the data store if copyWhenReading=false
In-Reply-To: <2819314.1203097567702.JavaMail.jira@brutus>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ 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.