[jira] [Commented] (MRESOLVER-370) Lock factory should dump lock states on failure
[ https://issues.apache.org/jira/browse/MRESOLVER-370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17731989#comment-17731989 ] ASF GitHub Bot commented on MRESOLVER-370: -- cstamas merged PR #299: URL: https://github.com/apache/maven-resolver/pull/299 > Lock factory should dump lock states on failure > --- > > Key: MRESOLVER-370 > URL: https://issues.apache.org/jira/browse/MRESOLVER-370 > Project: Maven Resolver > Issue Type: New Feature > Components: Resolver >Reporter: Tamas Cservenak >Assignee: Tamas Cservenak >Priority: Major > Fix For: 1.9.11 > > > When adapter "gives up" (as it could not acquire required lock), the error > currently states "how many retries were against which lock". This gives > information only about failed lock, but lacks the "whole picture". > Proposed change: in case of lock failure, factory should dump out all lock > states (may be huge on big builds), as that would allow simpler > identification of possible problems. All this could be sorted out at "high > level" (so no need to fiddle with file locks, hazelcast or redisson), but for > memory constraints it should NOT be enabled by default, this "diagnostic" > should be turned off by default, but possible by users to turn on if needed. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MRESOLVER-370) Lock factory should dump lock states on failure
[
https://issues.apache.org/jira/browse/MRESOLVER-370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17731938#comment-17731938
]
ASF GitHub Bot commented on MRESOLVER-370:
--
cstamas commented on code in PR #299:
URL: https://github.com/apache/maven-resolver/pull/299#discussion_r1227690609
##
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java:
##
@@ -32,18 +39,74 @@ public abstract class NamedLockSupport implements NamedLock
{
private final NamedLockFactorySupport factory;
+private final ConcurrentHashMap> diagnosticState; //
non-null only if diag enabled
Review Comment:
How would we list then ALL threads having locks?
> Lock factory should dump lock states on failure
> ---
>
> Key: MRESOLVER-370
> URL: https://issues.apache.org/jira/browse/MRESOLVER-370
> Project: Maven Resolver
> Issue Type: New Feature
> Components: Resolver
>Reporter: Tamas Cservenak
>Assignee: Tamas Cservenak
>Priority: Major
> Fix For: 1.9.11
>
>
> When adapter "gives up" (as it could not acquire required lock), the error
> currently states "how many retries were against which lock". This gives
> information only about failed lock, but lacks the "whole picture".
> Proposed change: in case of lock failure, factory should dump out all lock
> states (may be huge on big builds), as that would allow simpler
> identification of possible problems. All this could be sorted out at "high
> level" (so no need to fiddle with file locks, hazelcast or redisson), but for
> memory constraints it should NOT be enabled by default, this "diagnostic"
> should be turned off by default, but possible by users to turn on if needed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (MRESOLVER-370) Lock factory should dump lock states on failure
[
https://issues.apache.org/jira/browse/MRESOLVER-370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17731932#comment-17731932
]
ASF GitHub Bot commented on MRESOLVER-370:
--
gnodet commented on code in PR #299:
URL: https://github.com/apache/maven-resolver/pull/299#discussion_r1227686061
##
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java:
##
@@ -32,18 +39,74 @@ public abstract class NamedLockSupport implements NamedLock
{
private final NamedLockFactorySupport factory;
+private final ConcurrentHashMap> diagnosticState; //
non-null only if diag enabled
Review Comment:
Should we use a `ThreadLocal` ?
That would simplify the code a bit imho (untested):
```
private final ThreadLocal diagnosticState =
factory.isDiagnosticEnabled() ? ThreadLocal.withInitial(ArrayDeque::new) : null;
```
and then
```
if (diagnosticState != null) {
diagnosticState.get().push("shared");
}
```
> Lock factory should dump lock states on failure
> ---
>
> Key: MRESOLVER-370
> URL: https://issues.apache.org/jira/browse/MRESOLVER-370
> Project: Maven Resolver
> Issue Type: New Feature
> Components: Resolver
>Reporter: Tamas Cservenak
>Assignee: Tamas Cservenak
>Priority: Major
> Fix For: 1.9.11
>
>
> When adapter "gives up" (as it could not acquire required lock), the error
> currently states "how many retries were against which lock". This gives
> information only about failed lock, but lacks the "whole picture".
> Proposed change: in case of lock failure, factory should dump out all lock
> states (may be huge on big builds), as that would allow simpler
> identification of possible problems. All this could be sorted out at "high
> level" (so no need to fiddle with file locks, hazelcast or redisson), but for
> memory constraints it should NOT be enabled by default, this "diagnostic"
> should be turned off by default, but possible by users to turn on if needed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (MRESOLVER-370) Lock factory should dump lock states on failure
[
https://issues.apache.org/jira/browse/MRESOLVER-370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17731900#comment-17731900
]
ASF GitHub Bot commented on MRESOLVER-370:
--
cstamas commented on code in PR #299:
URL: https://github.com/apache/maven-resolver/pull/299#discussion_r1227565749
##
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java:
##
@@ -43,7 +43,7 @@ public abstract class NamedLockSupport implements NamedLock {
public NamedLockSupport(final String name, final NamedLockFactorySupport
factory) {
this.name = name;
this.factory = factory;
-this.state = factory.diagnostic ? new ConcurrentHashMap<>() : null;
+this.state = factory.isDiagnosticEnabled() ? new ConcurrentHashMap<>()
: null;
Review Comment:
Due this above (and absence of session, plus the life-span of factory and
lock instance across many sessions), I'd change "diag" switch just to be a Java
system property... @gnodet WDYT?
> Lock factory should dump lock states on failure
> ---
>
> Key: MRESOLVER-370
> URL: https://issues.apache.org/jira/browse/MRESOLVER-370
> Project: Maven Resolver
> Issue Type: New Feature
> Components: Resolver
>Reporter: Tamas Cservenak
>Assignee: Tamas Cservenak
>Priority: Major
> Fix For: 1.9.11
>
>
> When adapter "gives up" (as it could not acquire required lock), the error
> currently states "how many retries were against which lock". This gives
> information only about failed lock, but lacks the "whole picture".
> Proposed change: in case of lock failure, factory should dump out all lock
> states (may be huge on big builds), as that would allow simpler
> identification of possible problems. All this could be sorted out at "high
> level" (so no need to fiddle with file locks, hazelcast or redisson), but for
> memory constraints it should NOT be enabled by default, this "diagnostic"
> should be turned off by default, but possible by users to turn on if needed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (MRESOLVER-370) Lock factory should dump lock states on failure
[
https://issues.apache.org/jira/browse/MRESOLVER-370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17731732#comment-17731732
]
ASF GitHub Bot commented on MRESOLVER-370:
--
cstamas commented on code in PR #299:
URL: https://github.com/apache/maven-resolver/pull/299#discussion_r1227092805
##
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java:
##
@@ -43,7 +43,7 @@ public abstract class NamedLockSupport implements NamedLock {
public NamedLockSupport(final String name, final NamedLockFactorySupport
factory) {
this.name = name;
this.factory = factory;
-this.state = factory.diagnostic ? new ConcurrentHashMap<>() : null;
+this.state = factory.isDiagnosticEnabled() ? new ConcurrentHashMap<>()
: null;
Review Comment:
Am actually quite uneasy with this "logger level changes at runtime" As
named lock instances come and go, but this is possible as well:
```
DI container
|-|
NamedLockFactory (as is singleton)
|-|
Session1
|---|
Session2
| -|
Session3
| -|
NamedLock1
|---|
NamedLock2
|-|
```
As instances are cached (and ref counted), potentially they can span
multiple sessions even, without any connection to session. IF we would always
obey logger level, we could end up with "incomplete" diag data as well, as
logger debug may be disabled-enabled-disabled-enabled, but if lock instance
spans across all 4 changes, diag map would contain only part of data
> Lock factory should dump lock states on failure
> ---
>
> Key: MRESOLVER-370
> URL: https://issues.apache.org/jira/browse/MRESOLVER-370
> Project: Maven Resolver
> Issue Type: New Feature
> Components: Resolver
>Reporter: Tamas Cservenak
>Assignee: Tamas Cservenak
>Priority: Major
> Fix For: 1.9.11
>
>
> When adapter "gives up" (as it could not acquire required lock), the error
> currently states "how many retries were against which lock". This gives
> information only about failed lock, but lacks the "whole picture".
> Proposed change: in case of lock failure, factory should dump out all lock
> states (may be huge on big builds), as that would allow simpler
> identification of possible problems. All this could be sorted out at "high
> level" (so no need to fiddle with file locks, hazelcast or redisson), but for
> memory constraints it should NOT be enabled by default, this "diagnostic"
> should be turned off by default, but possible by users to turn on if needed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (MRESOLVER-370) Lock factory should dump lock states on failure
[
https://issues.apache.org/jira/browse/MRESOLVER-370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17731714#comment-17731714
]
ASF GitHub Bot commented on MRESOLVER-370:
--
gnodet commented on code in PR #299:
URL: https://github.com/apache/maven-resolver/pull/299#discussion_r1227027541
##
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java:
##
@@ -43,7 +43,7 @@ public abstract class NamedLockSupport implements NamedLock {
public NamedLockSupport(final String name, final NamedLockFactorySupport
factory) {
this.name = name;
this.factory = factory;
-this.state = factory.diagnostic ? new ConcurrentHashMap<>() : null;
+this.state = factory.isDiagnosticEnabled() ? new ConcurrentHashMap<>()
: null;
Review Comment:
You're right, it won't lead to a NPE, I misread the code. However, it's
caching the value of `isDiagnosticEnabled()`, so if we assume that its value
can change at runtime, we should create the map, then check again with
`factory.isDiagnosticEnabled()` in the code instead of `state != null`.
> Lock factory should dump lock states on failure
> ---
>
> Key: MRESOLVER-370
> URL: https://issues.apache.org/jira/browse/MRESOLVER-370
> Project: Maven Resolver
> Issue Type: New Feature
> Components: Resolver
>Reporter: Tamas Cservenak
>Assignee: Tamas Cservenak
>Priority: Major
> Fix For: 1.9.11
>
>
> When adapter "gives up" (as it could not acquire required lock), the error
> currently states "how many retries were against which lock". This gives
> information only about failed lock, but lacks the "whole picture".
> Proposed change: in case of lock failure, factory should dump out all lock
> states (may be huge on big builds), as that would allow simpler
> identification of possible problems. All this could be sorted out at "high
> level" (so no need to fiddle with file locks, hazelcast or redisson), but for
> memory constraints it should NOT be enabled by default, this "diagnostic"
> should be turned off by default, but possible by users to turn on if needed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (MRESOLVER-370) Lock factory should dump lock states on failure
[
https://issues.apache.org/jira/browse/MRESOLVER-370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17731690#comment-17731690
]
ASF GitHub Bot commented on MRESOLVER-370:
--
cstamas commented on code in PR #299:
URL: https://github.com/apache/maven-resolver/pull/299#discussion_r1226942522
##
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java:
##
@@ -43,7 +43,7 @@ public abstract class NamedLockSupport implements NamedLock {
public NamedLockSupport(final String name, final NamedLockFactorySupport
factory) {
this.name = name;
this.factory = factory;
-this.state = factory.diagnostic ? new ConcurrentHashMap<>() : null;
+this.state = factory.isDiagnosticEnabled() ? new ConcurrentHashMap<>()
: null;
Review Comment:
the method is used only here, all the rest checks are `state != null` (and
state is final), unsure how can this NPE
> Lock factory should dump lock states on failure
> ---
>
> Key: MRESOLVER-370
> URL: https://issues.apache.org/jira/browse/MRESOLVER-370
> Project: Maven Resolver
> Issue Type: New Feature
> Components: Resolver
>Reporter: Tamas Cservenak
>Assignee: Tamas Cservenak
>Priority: Major
> Fix For: 1.9.11
>
>
> When adapter "gives up" (as it could not acquire required lock), the error
> currently states "how many retries were against which lock". This gives
> information only about failed lock, but lacks the "whole picture".
> Proposed change: in case of lock failure, factory should dump out all lock
> states (may be huge on big builds), as that would allow simpler
> identification of possible problems. All this could be sorted out at "high
> level" (so no need to fiddle with file locks, hazelcast or redisson), but for
> memory constraints it should NOT be enabled by default, this "diagnostic"
> should be turned off by default, but possible by users to turn on if needed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (MRESOLVER-370) Lock factory should dump lock states on failure
[
https://issues.apache.org/jira/browse/MRESOLVER-370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17731691#comment-17731691
]
ASF GitHub Bot commented on MRESOLVER-370:
--
cstamas commented on code in PR #299:
URL: https://github.com/apache/maven-resolver/pull/299#discussion_r1226942522
##
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java:
##
@@ -43,7 +43,7 @@ public abstract class NamedLockSupport implements NamedLock {
public NamedLockSupport(final String name, final NamedLockFactorySupport
factory) {
this.name = name;
this.factory = factory;
-this.state = factory.diagnostic ? new ConcurrentHashMap<>() : null;
+this.state = factory.isDiagnosticEnabled() ? new ConcurrentHashMap<>()
: null;
Review Comment:
the method is used only here, in ctor, all the rest checks are `state !=
null` (and state is final), unsure how can this NPE
> Lock factory should dump lock states on failure
> ---
>
> Key: MRESOLVER-370
> URL: https://issues.apache.org/jira/browse/MRESOLVER-370
> Project: Maven Resolver
> Issue Type: New Feature
> Components: Resolver
>Reporter: Tamas Cservenak
>Assignee: Tamas Cservenak
>Priority: Major
> Fix For: 1.9.11
>
>
> When adapter "gives up" (as it could not acquire required lock), the error
> currently states "how many retries were against which lock". This gives
> information only about failed lock, but lacks the "whole picture".
> Proposed change: in case of lock failure, factory should dump out all lock
> states (may be huge on big builds), as that would allow simpler
> identification of possible problems. All this could be sorted out at "high
> level" (so no need to fiddle with file locks, hazelcast or redisson), but for
> memory constraints it should NOT be enabled by default, this "diagnostic"
> should be turned off by default, but possible by users to turn on if needed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (MRESOLVER-370) Lock factory should dump lock states on failure
[
https://issues.apache.org/jira/browse/MRESOLVER-370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17731688#comment-17731688
]
ASF GitHub Bot commented on MRESOLVER-370:
--
gnodet commented on code in PR #299:
URL: https://github.com/apache/maven-resolver/pull/299#discussion_r1226934933
##
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java:
##
@@ -43,7 +43,7 @@ public abstract class NamedLockSupport implements NamedLock {
public NamedLockSupport(final String name, final NamedLockFactorySupport
factory) {
this.name = name;
this.factory = factory;
-this.state = factory.diagnostic ? new ConcurrentHashMap<>() : null;
+this.state = factory.isDiagnosticEnabled() ? new ConcurrentHashMap<>()
: null;
Review Comment:
If the logger level changes at runtime, that may lead to an NPE, so I'd just
create the map in all cases.
> Lock factory should dump lock states on failure
> ---
>
> Key: MRESOLVER-370
> URL: https://issues.apache.org/jira/browse/MRESOLVER-370
> Project: Maven Resolver
> Issue Type: New Feature
> Components: Resolver
>Reporter: Tamas Cservenak
>Assignee: Tamas Cservenak
>Priority: Major
> Fix For: 1.9.11
>
>
> When adapter "gives up" (as it could not acquire required lock), the error
> currently states "how many retries were against which lock". This gives
> information only about failed lock, but lacks the "whole picture".
> Proposed change: in case of lock failure, factory should dump out all lock
> states (may be huge on big builds), as that would allow simpler
> identification of possible problems. All this could be sorted out at "high
> level" (so no need to fiddle with file locks, hazelcast or redisson), but for
> memory constraints it should NOT be enabled by default, this "diagnostic"
> should be turned off by default, but possible by users to turn on if needed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (MRESOLVER-370) Lock factory should dump lock states on failure
[
https://issues.apache.org/jira/browse/MRESOLVER-370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17731667#comment-17731667
]
ASF GitHub Bot commented on MRESOLVER-370:
--
gnodet commented on code in PR #299:
URL: https://github.com/apache/maven-resolver/pull/299#discussion_r1226865171
##
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockFactorySupport.java:
##
@@ -34,6 +35,8 @@
public abstract class NamedLockFactorySupport implements NamedLockFactory {
protected final Logger logger = LoggerFactory.getLogger(getClass());
+final boolean diagnostic = logger.isDebugEnabled();
Review Comment:
That's a bad idea. Loggers level can be updated at runtime, and the call to
`isDebugEnabled()` should be quite enough to not warrant caching the value.
##
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockFactorySupport.java:
##
@@ -34,6 +35,8 @@
public abstract class NamedLockFactorySupport implements NamedLockFactory {
protected final Logger logger = LoggerFactory.getLogger(getClass());
+final boolean diagnostic = logger.isDebugEnabled();
Review Comment:
That's a bad idea. Loggers level can be updated at runtime, and the call to
`isDebugEnabled()` should be fast enough to not warrant caching the value.
> Lock factory should dump lock states on failure
> ---
>
> Key: MRESOLVER-370
> URL: https://issues.apache.org/jira/browse/MRESOLVER-370
> Project: Maven Resolver
> Issue Type: New Feature
> Components: Resolver
>Reporter: Tamas Cservenak
>Assignee: Tamas Cservenak
>Priority: Major
> Fix For: 1.9.11
>
>
> When adapter "gives up" (as it could not acquire required lock), the error
> currently states "how many retries were against which lock". This gives
> information only about failed lock, but lacks the "whole picture".
> Proposed change: in case of lock failure, factory should dump out all lock
> states (may be huge on big builds), as that would allow simpler
> identification of possible problems. All this could be sorted out at "high
> level" (so no need to fiddle with file locks, hazelcast or redisson), but for
> memory constraints it should NOT be enabled by default, this "diagnostic"
> should be turned off by default, but possible by users to turn on if needed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (MRESOLVER-370) Lock factory should dump lock states on failure
[
https://issues.apache.org/jira/browse/MRESOLVER-370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17731617#comment-17731617
]
ASF GitHub Bot commented on MRESOLVER-370:
--
cstamas commented on code in PR #299:
URL: https://github.com/apache/maven-resolver/pull/299#discussion_r1226661563
##
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java:
##
@@ -32,18 +38,76 @@ public abstract class NamedLockSupport implements NamedLock
{
private final NamedLockFactorySupport factory;
+private final ConcurrentHashMap> state;
+
public NamedLockSupport(final String name, final NamedLockFactorySupport
factory) {
this.name = name;
this.factory = factory;
+this.state = factory.diagnostic ? new ConcurrentHashMap<>() : null;
}
@Override
public String name() {
return name;
}
+@Override
+public boolean lockShared(long time, TimeUnit unit) throws
InterruptedException {
+if (state != null) {
+state.computeIfAbsent(Thread.currentThread(), k -> new
ArrayDeque<>())
+.push("S");
Review Comment:
Agreed, although named locks API does not limit (boxing depth), in reality
syncContext does "shallow" boxing.
> Lock factory should dump lock states on failure
> ---
>
> Key: MRESOLVER-370
> URL: https://issues.apache.org/jira/browse/MRESOLVER-370
> Project: Maven Resolver
> Issue Type: Improvement
> Components: Resolver
>Reporter: Tamas Cservenak
>Priority: Major
>
> When adapter "gives up" (as it could not acquire required lock), the error
> currently states "how many retries were against which lock". This gives
> information only about failed lock, but lacks the "whole picture".
> Proposed change: in case of lock failure, factory should dump out all lock
> states (may be huge on big builds), as that would allow simpler
> identification of possible problems. All this could be sorted out at "high
> level" (so no need to fiddle with file locks, hazelcast or redisson), but for
> memory constraints it should NOT be enabled by default, this "diagnostic"
> should be turned off by default, but possible by users to turn on if needed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (MRESOLVER-370) Lock factory should dump lock states on failure
[
https://issues.apache.org/jira/browse/MRESOLVER-370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17731613#comment-17731613
]
ASF GitHub Bot commented on MRESOLVER-370:
--
gnodet commented on code in PR #299:
URL: https://github.com/apache/maven-resolver/pull/299#discussion_r1226656453
##
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockSupport.java:
##
@@ -32,18 +38,76 @@ public abstract class NamedLockSupport implements NamedLock
{
private final NamedLockFactorySupport factory;
+private final ConcurrentHashMap> state;
+
public NamedLockSupport(final String name, final NamedLockFactorySupport
factory) {
this.name = name;
this.factory = factory;
+this.state = factory.diagnostic ? new ConcurrentHashMap<>() : null;
}
@Override
public String name() {
return name;
}
+@Override
+public boolean lockShared(long time, TimeUnit unit) throws
InterruptedException {
+if (state != null) {
+state.computeIfAbsent(Thread.currentThread(), k -> new
ArrayDeque<>())
+.push("S");
Review Comment:
if we don't expect more than a few locks per thread at the same time, I
would use `shared` and `exclusive`, it will not require the reader to go back
to the source to understand what S/X means.
> Lock factory should dump lock states on failure
> ---
>
> Key: MRESOLVER-370
> URL: https://issues.apache.org/jira/browse/MRESOLVER-370
> Project: Maven Resolver
> Issue Type: Improvement
> Components: Resolver
>Reporter: Tamas Cservenak
>Priority: Major
>
> When adapter "gives up" (as it could not acquire required lock), the error
> currently states "how many retries were against which lock". This gives
> information only about failed lock, but lacks the "whole picture".
> Proposed change: in case of lock failure, factory should dump out all lock
> states (may be huge on big builds), as that would allow simpler
> identification of possible problems. All this could be sorted out at "high
> level" (so no need to fiddle with file locks, hazelcast or redisson), but for
> memory constraints it should NOT be enabled by default, this "diagnostic"
> should be turned off by default, but possible by users to turn on if needed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (MRESOLVER-370) Lock factory should dump lock states on failure
[
https://issues.apache.org/jira/browse/MRESOLVER-370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17731600#comment-17731600
]
ASF GitHub Bot commented on MRESOLVER-370:
--
gnodet commented on code in PR #299:
URL: https://github.com/apache/maven-resolver/pull/299#discussion_r1226637930
##
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/NamedLockFactory.java:
##
@@ -35,4 +37,18 @@ public interface NamedLockFactory {
* Performs a clean shut down of the factory.
*/
void shutdown();
+
+/**
+ * Utility method to provide more (factory specific) description when a
locking operation failed. Assumption is
+ * that provided list has at least one element (one failure) or more (in
case of retries), must not be {@code null}
+ * to get meaningful exceptions. The returned exception will be new
instance of {@link IllegalStateException} with
+ * passed in list added as suppressed exceptions. Still, the fact this
method has be invoked, means there is a
+ * "abort failure" ahead, so factory may either decorate (add info about
state) or even log some diagnostic
+ * about the state of the locks.
+ *
+ * @since TBD
+ * @return A new instance of {@link IllegalStateException} (decorated) and
may have other side effects as well
+ * (dumping state), never {@code null}.
+ */
+IllegalStateException failure(List attempts);
Review Comment:
If `NamedLockFactory` is a public SPI, maybe a `default` method would be
more appropriate here ?
> Lock factory should dump lock states on failure
> ---
>
> Key: MRESOLVER-370
> URL: https://issues.apache.org/jira/browse/MRESOLVER-370
> Project: Maven Resolver
> Issue Type: Improvement
> Components: Resolver
>Reporter: Tamas Cservenak
>Priority: Major
>
> When adapter "gives up" (as it could not acquire required lock), the error
> currently states "how many retries were against which lock". This gives
> information only about failed lock, but lacks the "whole picture".
> Proposed change: in case of lock failure, factory should dump out all lock
> states (may be huge on big builds), as that would allow simpler
> identification of possible problems. All this could be sorted out at "high
> level" (so no need to fiddle with file locks, hazelcast or redisson), but for
> memory constraints it should NOT be enabled by default, this "diagnostic"
> should be turned off by default, but possible by users to turn on if needed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[jira] [Commented] (MRESOLVER-370) Lock factory should dump lock states on failure
[ https://issues.apache.org/jira/browse/MRESOLVER-370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17731599#comment-17731599 ] ASF GitHub Bot commented on MRESOLVER-370: -- cstamas commented on PR #299: URL: https://github.com/apache/maven-resolver/pull/299#issuecomment-1587302862 Example outputs https://gist.github.com/cstamas/fe1bd5b73c3e02877d9647c00aa40831 > Lock factory should dump lock states on failure > --- > > Key: MRESOLVER-370 > URL: https://issues.apache.org/jira/browse/MRESOLVER-370 > Project: Maven Resolver > Issue Type: Improvement > Components: Resolver >Reporter: Tamas Cservenak >Priority: Major > > When adapter "gives up" (as it could not acquire required lock), the error > currently states "how many retries were against which lock". This gives > information only about failed lock, but lacks the "whole picture". > Proposed change: in case of lock failure, factory should dump out all lock > states (may be huge on big builds), as that would allow simpler > identification of possible problems. All this could be sorted out at "high > level" (so no need to fiddle with file locks, hazelcast or redisson), but for > memory constraints it should NOT be enabled by default, this "diagnostic" > should be turned off by default, but possible by users to turn on if needed. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (MRESOLVER-370) Lock factory should dump lock states on failure
[ https://issues.apache.org/jira/browse/MRESOLVER-370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17731598#comment-17731598 ] ASF GitHub Bot commented on MRESOLVER-370: -- cstamas opened a new pull request, #299: URL: https://github.com/apache/maven-resolver/pull/299 The change "push" all `NamedLock` interface methods "level down" on implementations, to be able to augment instances at `NamedLockSupport` level. If used factory DEBUG level is enabled on logger, it turns on "diagnostic" mode (will consume more memory as well!): * factory makes all named lock instances to gather "diag stats" (memory!) * on adapter failure, factory will dump out diag state, * each active lock (non closed) will dump it's state out * output goes to lock factory DEBUG logger, but it may be huge By default, when "diagnostic" not turned on, no change, merely the construction of finally thrown ISEx is relocated from adapter to factory. --- https://issues.apache.org/jira/browse/MRESOLVER-370 > Lock factory should dump lock states on failure > --- > > Key: MRESOLVER-370 > URL: https://issues.apache.org/jira/browse/MRESOLVER-370 > Project: Maven Resolver > Issue Type: Improvement > Components: Resolver >Reporter: Tamas Cservenak >Priority: Major > > When adapter "gives up" (as it could not acquire required lock), the error > currently states "how many retries were against which lock". This gives > information only about failed lock, but lacks the "whole picture". > Proposed change: in case of lock failure, factory should dump out all lock > states (may be huge on big builds), as that would allow simpler > identification of possible problems. All this could be sorted out at "high > level" (so no need to fiddle with file locks, hazelcast or redisson), but for > memory constraints it should NOT be enabled by default, this "diagnostic" > should be turned off by default, but possible by users to turn on if needed. -- This message was sent by Atlassian Jira (v8.20.10#820010)
