[jira] Commented: (JCR-890) concurrent read-only access to a session
[ https://issues.apache.org/jira/browse/JCR-890?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12881593#action_12881593 ] Jukka Zitting commented on JCR-890: --- The SessionOperation mechanism is meant to synchronize only session-related JCR API calls, so unlike with XA transactions there should never be a need for two threads to use the same lock concurrently. I'm using a java.util.concurrent lock instead of a synchronized block only to enable more accurate logging of potential concurrency problems. > concurrent read-only access to a session > > > Key: JCR-890 > URL: https://issues.apache.org/jira/browse/JCR-890 > Project: Jackrabbit Content Repository > Issue Type: Improvement > Components: jackrabbit-core >Reporter: David Nuescheler >Assignee: Jukka Zitting > Fix For: 2.2.0 > > Attachments: session-class-move-norename.patch, > session-class-move.patch > > > Even though the JCR specification does not make a statement about Sessions > shared across a number of threads I think it would be great for many > applications if we could state that sharing a read-only session is supported > by Jackrabbit. > On many occasions in the mailing lists we stated that there should not be an > issue with sharing a read-only session, however I think it has never been > thoroughly tested or even specified as a "design goal". > If we can come to an agreement that this is desirable I think it would be > great to start including testcases to validate that behaviour and update the > documentation respectively. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-890) concurrent read-only access to a session
[ https://issues.apache.org/jira/browse/JCR-890?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12881557#action_12881557 ] Claus Köll commented on JCR-890: Hi Jukka, I saw your commits with the new SessionOperations. First good work but i want to let you know that the locking is only Thread based and we will get some problems in a xa environment on a applicationserver because the reentrant calls must not come from the same thread. I would prefere to handle now the whole locking semantic in our own classes to handle this behaviours in a global way. See JCR-2089 > concurrent read-only access to a session > > > Key: JCR-890 > URL: https://issues.apache.org/jira/browse/JCR-890 > Project: Jackrabbit Content Repository > Issue Type: Improvement > Components: jackrabbit-core >Reporter: David Nuescheler >Assignee: Jukka Zitting > Fix For: 2.2.0 > > Attachments: session-class-move-norename.patch, > session-class-move.patch > > > Even though the JCR specification does not make a statement about Sessions > shared across a number of threads I think it would be great for many > applications if we could state that sharing a read-only session is supported > by Jackrabbit. > On many occasions in the mailing lists we stated that there should not be an > issue with sharing a read-only session, however I think it has never been > thoroughly tested or even specified as a "design goal". > If we can come to an agreement that this is desirable I think it would be > great to start including testcases to validate that behaviour and update the > documentation respectively. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-890) concurrent read-only access to a session
[ https://issues.apache.org/jira/browse/JCR-890?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12871131#action_12871131 ] Jukka Zitting commented on JCR-890: --- See JCR-2640 for a proposed way (and initial patch) to get rid of the troublesome getters. > concurrent read-only access to a session > > > Key: JCR-890 > URL: https://issues.apache.org/jira/browse/JCR-890 > Project: Jackrabbit Content Repository > Issue Type: Improvement > Components: jackrabbit-core >Reporter: David Nuescheler >Assignee: Jukka Zitting > Fix For: 2.2.0 > > Attachments: session-class-move-norename.patch, > session-class-move.patch > > > Even though the JCR specification does not make a statement about Sessions > shared across a number of threads I think it would be great for many > applications if we could state that sharing a read-only session is supported > by Jackrabbit. > On many occasions in the mailing lists we stated that there should not be an > issue with sharing a read-only session, however I think it has never been > thoroughly tested or even specified as a "design goal". > If we can come to an agreement that this is desirable I think it would be > great to start including testcases to validate that behaviour and update the > documentation respectively. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-890) concurrent read-only access to a session
[ https://issues.apache.org/jira/browse/JCR-890?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12869170#action_12869170 ] Stefan Guggisberg commented on JCR-890: --- i don't have a strong opinion regarding the suggested refactoring in general. if it enables a better tool support, that's ok with me. however, i do share angela's concerns regarding changing the visibility of internal classes/methods. since jukka said that he will address those concerns in another patch, i'll wait for the next patch. > concurrent read-only access to a session > > > Key: JCR-890 > URL: https://issues.apache.org/jira/browse/JCR-890 > Project: Jackrabbit Content Repository > Issue Type: Improvement > Components: jackrabbit-core >Reporter: David Nuescheler >Assignee: Jukka Zitting > Fix For: 2.2.0 > > Attachments: session-class-move-norename.patch, > session-class-move.patch > > > Even though the JCR specification does not make a statement about Sessions > shared across a number of threads I think it would be great for many > applications if we could state that sharing a read-only session is supported > by Jackrabbit. > On many occasions in the mailing lists we stated that there should not be an > issue with sharing a read-only session, however I think it has never been > thoroughly tested or even specified as a "design goal". > If we can come to an agreement that this is desirable I think it would be > great to start including testcases to validate that behaviour and update the > documentation respectively. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-890) concurrent read-only access to a session
[ https://issues.apache.org/jira/browse/JCR-890?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12868623#action_12868623 ] Jukka Zitting commented on JCR-890: --- Thanks for the review! > why is the package called 'session'? Perhaps some other term than "session" would be clearer. The way I see the current Jackrabbit architecture (as muddy as it at times is), there are basically two main parts: 1) "repository" part that consists of the global persisted state stored in persistence managers, node type registry, etc. This state is seen and shared by all clients that access the repository. 2) "session" part that consist of the transient changes that have not yet been persisted. This part is private to each client. Note that I also consider things like immediately persisted workspace operations (imports, copies and moves, etc.) as part of the "session" layer up to the point when the relevant ChangeLog gets persisted. Thus I count also classes like BatchedItemOperations as part of this architectural layer. Would "global" and "local" be better terms? Note that I'd rather avoid labels like "persisted" and "transient" to prevent confusion with things like JCR-2051 (and since "transient" is a reserved word in Java). Basically, when I talk about "session-related classes" above, I mean code related to point 2 above. > visibility I was planning to move these classes first so I'd have a better picture of these package-private dependencies (as seen in session-class-move-norename.patch) before refactoring them away. Note that in most cases I expect these changes to be minor refactorings, like replacing a call to RepositoryImpl.getSomething() with an injected dependency. But yes, I can take care of this also before other changes. > concurrent read-only access to a session > > > Key: JCR-890 > URL: https://issues.apache.org/jira/browse/JCR-890 > Project: Jackrabbit Content Repository > Issue Type: Improvement > Components: jackrabbit-core >Reporter: David Nuescheler >Assignee: Jukka Zitting > Fix For: 2.2.0 > > Attachments: session-class-move-norename.patch, > session-class-move.patch > > > Even though the JCR specification does not make a statement about Sessions > shared across a number of threads I think it would be great for many > applications if we could state that sharing a read-only session is supported > by Jackrabbit. > On many occasions in the mailing lists we stated that there should not be an > issue with sharing a read-only session, however I think it has never been > thoroughly tested or even specified as a "design goal". > If we can come to an agreement that this is desirable I think it would be > great to start including testcases to validate that behaviour and update the > documentation respectively. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-890) concurrent read-only access to a session
[
https://issues.apache.org/jira/browse/JCR-890?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12868599#action_12868599
]
angela commented on JCR-890:
my comments from a quick first glance at the patches:
why is the package called 'session'? from your explanation on the dev list
("[...] to move most of the session-related classes from o.a.j.core to
a new o.a.j.core.session package [...]") i would have expected that this is
related to either JCR API implementations in close relationship to
the Session or modifications on the Session level (aka transient modifications).
but now the patch moves classes that i wouldn't have expected to be affected
based your explanation: BatchedItemOperations (purely workspace operations on
the item state level), ItemData and subclasses (really internal stuff) just to
mention 2 of them. what was the design behind those moves?
second i have strong concern regarding changing the visibility of so many
classes and methods that i consider to be repository internals.
making them all public feels really wrong to me what exactly do you mean by
"My plan is to refactor these cases so that we don't expose more of the
Jackrabbit internals to client code."? if you plan major refactoring of other
core classes not affected by the move, why not addressing this first before
moving classes that from my point of view don't belong to the target package?
-1 for the current patch.
> concurrent read-only access to a session
>
>
> Key: JCR-890
> URL: https://issues.apache.org/jira/browse/JCR-890
> Project: Jackrabbit Content Repository
> Issue Type: Improvement
> Components: jackrabbit-core
>Reporter: David Nuescheler
>Assignee: Jukka Zitting
> Fix For: 2.2.0
>
> Attachments: session-class-move-norename.patch,
> session-class-move.patch
>
>
> Even though the JCR specification does not make a statement about Sessions
> shared across a number of threads I think it would be great for many
> applications if we could state that sharing a read-only session is supported
> by Jackrabbit.
> On many occasions in the mailing lists we stated that there should not be an
> issue with sharing a read-only session, however I think it has never been
> thoroughly tested or even specified as a "design goal".
> If we can come to an agreement that this is desirable I think it would be
> great to start including testcases to validate that behaviour and update the
> documentation respectively.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-890) concurrent read-only access to a session
[
https://issues.apache.org/jira/browse/JCR-890?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12524261
]
Thomas Mueller commented on JCR-890:
In a many cases, parameters can be processed outside the synchronized block.
Also, I would catch throwables and convert them to RepositoryExceptions, or at
least log them:
public void someMethod(parameters) throws RepositoryException {
try {
log(...)
prepare parameters
synchronized (session) {
// do something
}
} catch (Throwable e) {
throw logAndConvert(e);
}
}
> effort to look at synchronization within the lower levels of Jackrabbit?
Why not. In my view testing with specific use cases / benchmarks helps a lot to
find improvements.
Thomas
> concurrent read-only access to a session
>
>
> Key: JCR-890
> URL: https://issues.apache.org/jira/browse/JCR-890
> Project: Jackrabbit
> Issue Type: Improvement
> Components: core
>Reporter: David Nuescheler
>Assignee: Stefan Guggisberg
>
> Even though the JCR specification does not make a statement about Sessions
> shared across a number of threads I think it would be great for many
> applications if we could state that sharing a read-only session is supported
> by Jackrabbit.
> On many occasions in the mailing lists we stated that there should not be an
> issue with sharing a read-only session, however I think it has never been
> thoroughly tested or even specified as a "design goal".
> If we can come to an agreement that this is desirable I think it would be
> great to start including testcases to validate that behaviour and update the
> documentation respectively.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-890) concurrent read-only access to a session
[ https://issues.apache.org/jira/browse/JCR-890?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12524166 ] Padraic Hannon commented on JCR-890: I think this is similar to the discussion from JCR-1050. Perhaps we need a larger effort to look at synchronization within the lower levels of Jackrabbit? > concurrent read-only access to a session > > > Key: JCR-890 > URL: https://issues.apache.org/jira/browse/JCR-890 > Project: Jackrabbit > Issue Type: Improvement > Components: core >Reporter: David Nuescheler >Assignee: Stefan Guggisberg > > Even though the JCR specification does not make a statement about Sessions > shared across a number of threads I think it would be great for many > applications if we could state that sharing a read-only session is supported > by Jackrabbit. > On many occasions in the mailing lists we stated that there should not be an > issue with sharing a read-only session, however I think it has never been > thoroughly tested or even specified as a "design goal". > If we can come to an agreement that this is desirable I think it would be > great to start including testcases to validate that behaviour and update the > documentation respectively. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-890) concurrent read-only access to a session
[
https://issues.apache.org/jira/browse/JCR-890?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12524130
]
Jukka Zitting commented on JCR-890:
---
So you'd have something like this on all non-trivial API methods:
/** The session this object is associated with */
private final Session session;
public void someMethod() {
synchronized (session) {
// do something
}
}
I like the idea.
> concurrent read-only access to a session
>
>
> Key: JCR-890
> URL: https://issues.apache.org/jira/browse/JCR-890
> Project: Jackrabbit
> Issue Type: Improvement
> Components: core
>Reporter: David Nuescheler
>Assignee: Stefan Guggisberg
>
> Even though the JCR specification does not make a statement about Sessions
> shared across a number of threads I think it would be great for many
> applications if we could state that sharing a read-only session is supported
> by Jackrabbit.
> On many occasions in the mailing lists we stated that there should not be an
> issue with sharing a read-only session, however I think it has never been
> thoroughly tested or even specified as a "design goal".
> If we can come to an agreement that this is desirable I think it would be
> great to start including testcases to validate that behaviour and update the
> documentation respectively.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-890) concurrent read-only access to a session
[ https://issues.apache.org/jira/browse/JCR-890?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12524073 ] Thomas Mueller commented on JCR-890: Write access within the same session should also be synchronized. But read access is more important. > concurrent read-only access to a session > > > Key: JCR-890 > URL: https://issues.apache.org/jira/browse/JCR-890 > Project: Jackrabbit > Issue Type: Improvement > Components: core >Reporter: David Nuescheler >Assignee: Stefan Guggisberg > > Even though the JCR specification does not make a statement about Sessions > shared across a number of threads I think it would be great for many > applications if we could state that sharing a read-only session is supported > by Jackrabbit. > On many occasions in the mailing lists we stated that there should not be an > issue with sharing a read-only session, however I think it has never been > thoroughly tested or even specified as a "design goal". > If we can come to an agreement that this is desirable I think it would be > great to start including testcases to validate that behaviour and update the > documentation respectively. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-890) concurrent read-only access to a session
[ https://issues.apache.org/jira/browse/JCR-890?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12524070 ] Thomas Mueller commented on JCR-890: +1 Robustness is very important in my view. Even if it has an effect on performance (I think it does not, but we could measure it). > concurrent read-only access to a session > > > Key: JCR-890 > URL: https://issues.apache.org/jira/browse/JCR-890 > Project: Jackrabbit > Issue Type: Improvement > Components: core >Reporter: David Nuescheler >Assignee: Stefan Guggisberg > > Even though the JCR specification does not make a statement about Sessions > shared across a number of threads I think it would be great for many > applications if we could state that sharing a read-only session is supported > by Jackrabbit. > On many occasions in the mailing lists we stated that there should not be an > issue with sharing a read-only session, however I think it has never been > thoroughly tested or even specified as a "design goal". > If we can come to an agreement that this is desirable I think it would be > great to start including testcases to validate that behaviour and update the > documentation respectively. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-890) concurrent read-only access to a session
[ https://issues.apache.org/jira/browse/JCR-890?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12524065 ] Marcel Reutegger commented on JCR-890: -- I hereby withdraw my proposal and state another one: Move all synchronization in Jackrabbit that deals with concurrent read to the classes that implement the JCR API. In other words, make Jackrabbit entirely thread-safe for a client using the API. This includes all interfaces and not just the Repository. Why did I change my mind? With a wrapper approach a client may (by mistake) still use Jackrabbit 'directly' and then use a single session instance from concurrent threads. So far we just say: well, that's not supported, so it's your fault. If JCR is supposed to be infrastructure than an implementation should be robust and must not break just because some code does not behave as it should. I even think the performance will be better compared to the current jackrabbit-core because we currently have many low level data structures that need to be synchronized. If we decide to synchronize on a coarser grained level like the API we can get rid of those low level synchronizations and as a result there will be less synchronized blocks that are entered. > concurrent read-only access to a session > > > Key: JCR-890 > URL: https://issues.apache.org/jira/browse/JCR-890 > Project: Jackrabbit > Issue Type: Improvement > Components: core >Reporter: David Nuescheler >Assignee: Stefan Guggisberg > > Even though the JCR specification does not make a statement about Sessions > shared across a number of threads I think it would be great for many > applications if we could state that sharing a read-only session is supported > by Jackrabbit. > On many occasions in the mailing lists we stated that there should not be an > issue with sharing a read-only session, however I think it has never been > thoroughly tested or even specified as a "design goal". > If we can come to an agreement that this is desirable I think it would be > great to start including testcases to validate that behaviour and update the > documentation respectively. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-890) concurrent read-only access to a session
[ https://issues.apache.org/jira/browse/JCR-890?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12496251 ] Marcel Reutegger commented on JCR-890: -- I agree that this 'feature' is desirable however I would rather implement sharing read only session in a decoration layer or some sort of proxy on top of the JCR repository. We already have enough synchronization statements in the code and introducing even more will slow down clients that are not interested to share a session, e.g. a client that writes to a session would also be heavily synchronized, which is unnecessary in this case. I think we should choose a similar approach like the collections framework in java. All collection implementations are not thread-safe (except the very old ones from java 1.0 and 1.1), but there are wrapper classes that make them thread-safe if the client wishes so. This approach also has the advantage that the wrapper/decorator/proxy can throw an exception if a client still tries to call a method that writes to the repository. If we build the thread-safeness into the jackrabbit core we don't have this control and clients could still write to a session that is shared. I therefore propose the following: - remove all synchronization in the core that is meant to support concurrent read access to a session - create a read only decorator that synchronizes access to session someting along the lines: Session session = Util.synchronizedSession(session); // use session from multiple threads > concurrent read-only access to a session > > > Key: JCR-890 > URL: https://issues.apache.org/jira/browse/JCR-890 > Project: Jackrabbit > Issue Type: Improvement > Components: core >Reporter: David Nuescheler > Assigned To: Stefan Guggisberg > > Even though the JCR specification does not make a statement about Sessions > shared across a number of threads I think it would be great for many > applications if we could state that sharing a read-only session is supported > by Jackrabbit. > On many occasions in the mailing lists we stated that there should not be an > issue with sharing a read-only session, however I think it has never been > thoroughly tested or even specified as a "design goal". > If we can come to an agreement that this is desirable I think it would be > great to start including testcases to validate that behaviour and update the > documentation respectively. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
