[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17730860#comment-17730860
]
Julian Reschke commented on OAK-7259:
-
trunk: (1.9.0)
[1332a099c8|https://github.com/apache/jackrabbit-oak/commit/1332a099c85853d0313beab53165453f9a4c65ff]
[2e069db292|https://github.com/apache/jackrabbit-oak/commit/2e069db2927197658a2304c0dd097e05e8db1bce]
[ddca5b1db4|https://github.com/apache/jackrabbit-oak/commit/ddca5b1db44165e18c7e071ddf3d54b2f473d712]
[f71491fdb6|https://github.com/apache/jackrabbit-oak/commit/f71491fdb66ee42d6912952f42b1044d34048b07]
1.22: (1.9.0)
[1332a099c8|https://github.com/apache/jackrabbit-oak/commit/1332a099c85853d0313beab53165453f9a4c65ff]
[2e069db292|https://github.com/apache/jackrabbit-oak/commit/2e069db2927197658a2304c0dd097e05e8db1bce]
[ddca5b1db4|https://github.com/apache/jackrabbit-oak/commit/ddca5b1db44165e18c7e071ddf3d54b2f473d712]
[f71491fdb6|https://github.com/apache/jackrabbit-oak/commit/f71491fdb66ee42d6912952f42b1044d34048b07]
...in retired branches:
1.10: (1.9.0)
[1332a099c8|https://github.com/apache/jackrabbit-oak/commit/1332a099c85853d0313beab53165453f9a4c65ff]
[2e069db292|https://github.com/apache/jackrabbit-oak/commit/2e069db2927197658a2304c0dd097e05e8db1bce]
[ddca5b1db4|https://github.com/apache/jackrabbit-oak/commit/ddca5b1db44165e18c7e071ddf3d54b2f473d712]
[f71491fdb6|https://github.com/apache/jackrabbit-oak/commit/f71491fdb66ee42d6912952f42b1044d34048b07]
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10.0
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16392609#comment-16392609
]
Michael Dürig commented on OAK-7259:
The gist of mentioned discussions was that embedding it reduces downstream
dependencies and complexity. When need arises we can still change the
dependency scope.
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16391379#comment-16391379
]
Andrei Dulceanu commented on OAK-7259:
--
Per offline discussion with [~frm], [~mduerig] and [~volteanu], I removed the
dependency from all other modules in which it was introduced and instead
embedded it in {{oak-segment-tar}}. Done at r1826245.
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16389342#comment-16389342
]
Andrei Dulceanu commented on OAK-7259:
--
Added missing dependencies in {{oak-it-osgi}} at r1825996 and r1826091.
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16389040#comment-16389040
]
Julian Reschke commented on OAK-7259:
-
trunk: [r1825996|http://svn.apache.org/r1825996]
[r1825984|http://svn.apache.org/r1825984]
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16387932#comment-16387932
]
Marcel Reutegger commented on OAK-7259:
---
Jenkins also failed. See OAK-7311.
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16386246#comment-16386246
]
Michael Dürig commented on OAK-7259:
+1, looks good AFAICS.
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16386119#comment-16386119
]
Andrei Dulceanu commented on OAK-7259:
--
[~mduerig], thanks for clarifying this! At [0] you can find the outcome of our
discussions. IMO, all previous points are covered. Still couldn't do much about
the {{OpenDataException}}, but at least we're good (now) in case those maps
don't contain any items. If you are ok with it, I'll go ahead and commit it.
/cc [~frm]
[0] https://github.com/dulceanu/jackrabbit-oak/commits/issues/OAK-7259
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16386008#comment-16386008
]
Michael Dürig commented on OAK-7259:
Unless I missed it there is no getter. AFAIK without a getter this will surface
as an operation rather as a property.
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16385926#comment-16385926
]
Andrei Dulceanu commented on OAK-7259:
--
[~mduerig], isn't this already covered in [0]? That's why I asked you about the
property with getter/setter in my previous comment...
[0]
https://github.com/dulceanu/jackrabbit-oak/blob/issues/OAK-7259/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreStats.java#L115
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16385830#comment-16385830
]
Michael Dürig commented on OAK-7259:
AFAICS the only missing bit is a setter on the MBean and looping it through all
the way to the {{CommitsTracker.collectStackTraces}} flag.
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16385648#comment-16385648
]
Andrei Dulceanu commented on OAK-7259:
--
{quote}
Also we should probably make SegmentNodeStoreStats.setCollectStackTraces a
property as this makes it easier to use in most JMX clients.
{quote}
[~mduerig], you mean having a new instance variable in
{{SegmentNodeStoreStats}}, with getter/setter?
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16381732#comment-16381732
]
Michael Dürig commented on OAK-7259:
[~dulceanu], I like the new patch! It addresses all my concerns. IMO it's fit
for trunk.
Re. {{OpenDataException}}: An empty table is nothing exceptional, that's why I
think it is wrong to throw an exception in this case. If there is no way to
return an empty table, can we return an explicitly empty table (i.e. just a
single row containing dashes or so)? Or would this just make matters worse?
Somewhat related, it might be better to return the raw data from
{{CommitsTracker}} and do the conversion to {{TabularData}} to the MBean
implementation. This would also simplify testability of {{CommitsTracker}}.
Also we should probably make {{SegmentNodeStoreStats.setCollectStackTraces}} a
property as this makes it easier to use in most JMX clients.
Finally in the case when collecting stack traces is turned off, AFIU the map of
queued writers will contain the thread's name both as key and value. Wouldn't
it be clearer to just put "NA" as values?
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16380443#comment-16380443
]
Andrei Dulceanu commented on OAK-7259:
--
In the branch previously shared [0], I made the following changes:
* extracted all the logic related to tracking writers in its own class,
{{CommitsTracker}}
* delegated logic for keeping last x writers to {{ConcurrentLinkedHashMap}}
(introduced dependency to {{com.googlecode.concurrentlinkedhashmap}} in
{{oak-parent}}; do we need to move this to {{oak-segment-tar}} only?)
* unified {{SegmentNodeStoreStats.dequeuedAfter()}} and
{{SegmentNodeStoreStats.onCommitDequeued()}} into a single method, as well as
{{SegmentNodeStoreStats.onCommit}} and SegmentNodeStoreStats.committedAfter()
{quote}
I would prefer to return an empty table from
SegmentNodeStoreStatsMBean.getCommitsCountPerWriter() and
SegmentNodeStoreStatsMBean.getQueuedWriters() instead of throwing an
OpenDataException.
{quote}
I couldn't find a reasonable solution for this, since having an empty {{data}}
map in either of the methods will result in an {{IllegalArgumentException}}
being raised. Any suggestions on how to avoid this?
{quote}
Just an idea for now, can (should?) we make
SegmentNodeStoreStats.commitsCountMapMaxSize configurable through a setter in
SegmentNodeStoreStatsMBean?
{quote}
The previous, custom logic for bookkeeping allowed changing the max size on the
fly, but switching to {{ConcurrentLinkedHashMap}} we lost this flexibility. If
this is truly needed, we could revisit the code (in a new issue?).
[~frm], [~mduerig] WDYT?
[0] https://github.com/dulceanu/jackrabbit-oak/tree/issues/OAK-7259
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16378532#comment-16378532
]
Michael Dürig commented on OAK-7259:
I share [~frm] concerns re. {{updateCommitsCountMap()}} not being thread safe
and I think this is not comparable with {{ConcurrentHashMap}}. Concurrent calls
to {{updateCommitsCountMap()}} might lead to an incoherent state that is non
transient. However, I also share [~dulceanu] concerns re. adding additional
potential contention points on the "commit path". As an alternative, couldn't
we move the chore of updating {{commitsCountMap}} to the consumer thread
calling {{SegmentNodeStoreStats.getCommitsCountPerWriter()}}? My idea would be
to queue pending {{writernNames}} s that would otherwise be directly passed to
{{updateCommitsCountMap()}} and batch process them in calls to
{{getCommitsCountPerWriter()}}. IMO a good starting point would be to move all
related logic to its own class as [~frm] suggested as well.
>From the peanut gallery:
* {{Thread.getStackTrace()}} is expensive. We need to understand whether this
is problematic in this context or not. Starting with Java 9 there is
{{StackWalker}}, which is supposed to be cheaper. Until then, we should
probably have a way to switch collecting stack traces off.
* Can we unify {{SegmentNodeStoreStats.dequeuedAfter()}} and
{{SegmentNodeStoreStats.onCommitDequeued()}}?
* I would prefer to return an empty table from
{{SegmentNodeStoreStatsMBean.getCommitsCountPerWriter()}} and
{{SegmentNodeStoreStatsMBean.getQueuedWriters()}} instead of throwing an
{{OpenDataException}}.
* Just an idea for now, can (should?) we make
{{SegmentNodeStoreStats.commitsCountMapMaxSize}} configurable through a setter
in {{SegmentNodeStoreStatsMBean}}?
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16372562#comment-16372562
]
Francesco Mari commented on OAK-7259:
-
{quote}Just writing this, I realized that SegmentNodeStoreStats#onCommit is
only called with the commit semaphore acquired so it won't need additional
synchronization (in the current implementation of LockBasedScheduler). As for
onCommitQueued and onCommitDequeued, going forward with the current approach
means that getQueuedWriters might return an incomplete view of the currently
queued threads, which should be ok, since this is only about monitoring.{quote}
If no other thread is reading those data structures, every thread passing by
{{onCommit}} will find the lock free. This will not have a noticeable
performance overhead for the threads in the commit queue. Thread-safety is
important to not to report inconsistent data when both reader and writer
threads access the data structure.
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16365769#comment-16365769
]
Andrei Dulceanu commented on OAK-7259:
--
[~frm], while I agree with your concern about thread-safety for these newly
introduced methods, I think we should take into consideration the purpose of
{{SegmentNodeStoreStats}} which is monitoring. I guess the following excerpt
from {{ConcurrentHashMap}}'s JavaDoc applies here as well:
{quote}
Otherwise the results of these methods reflect transient states that may be
adequate for monitoring or estimation purposes, but not for program control.
{quote}
What I wanted to avoid was to introduce yet another lock contention point.
Suppose multiple threads access {{LockBasedScheduler#schedule}} while the
commit semaphore is already acquired by some other thread. If we go for a
synchronized version of {{SegmentNodeStoreStats#getCommitsCountPerWriter}}, as
found in [0], we'll basically have all those threads contend for the newly
introduced lock. Moreover, if the thread which acquired the semaphore modifies
(with the same lock held) the data structures used in
{{#getCommitsCountPerWriter}}, then we basically block all other threads.
Just writing this, I realized that {{SegmentNodeStoreStats#onCommit}} is only
called with the commit semaphore acquired so it won't need additional
synchronization (in the current implementation of {{LockBasedScheduler}}). As
for {{onCommitQueued}} and {{onCommitDequeued}}, going forward with the current
approach means that {{getQueuedWriters}} might return an incomplete view of the
currently queued threads, which should be ok, since this is only about
monitoring.
WDYT?
[0]
https://github.com/apache/jackrabbit-oak/blob/trunk/oak-store-composite/src/main/java/org/apache/jackrabbit/oak/composite/CompositeNodeStoreStats.java#L150
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16365694#comment-16365694
]
Francesco Mari commented on OAK-7259:
-
[~dulceanu], the foundation of the patch looks good to me, but I'm concerned
about {{SegmentNodeStoreStats#updateCommitsCountMap}} and, in general, about
the accesses to {{uniqueWriters}}, {{orderedWriters}}, and to the newly
introduced fields of {{SegmentNodeStoreStats}}. While those fields are
thread-safe when taken singularly, they are not thread-safe when used together.
For some concurrent access to those fields, some constraints on the data might
be violated. I suggest encapsulating these fields in another class and making
this new class thread-safe. This should make it easier to reason about how the
constraints hold under multi-threaded access.
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OAK-7259) Improve SegmentNodeStoreStats to include number of commits per thread and threads currently waiting on the semaphore
[
https://issues.apache.org/jira/browse/OAK-7259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16365603#comment-16365603
]
Andrei Dulceanu commented on OAK-7259:
--
At [0], there's a first implementation of the ideas discussed in the
description. [~frm], could you take a look at it and share your feedback?
/cc [~mduerig]
[0] https://github.com/dulceanu/jackrabbit-oak/tree/issues/OAK-7259
> Improve SegmentNodeStoreStats to include number of commits per thread and
> threads currently waiting on the semaphore
>
>
> Key: OAK-7259
> URL: https://issues.apache.org/jira/browse/OAK-7259
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: segment-tar
>Reporter: Andrei Dulceanu
>Assignee: Andrei Dulceanu
>Priority: Major
> Labels: tooling
> Fix For: 1.9.0, 1.10
>
>
> When investigating the performance of {{segment-tar}}, the source of the
> writes (commits) is a very useful indicator of the cause.
> To better understand which threads are currently writing in the repository
> and which are blocked on the semaphore, we need to improve
> {{SegmentNodeStoreStats}} to:
> * expose the number of commits executed per thread
> * expose threads currently waiting on the semaphore
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
