You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/07/22 19:05:05 UTC
[GitHub] [accumulo] EdColeman opened a new pull request, #2824: Spotbugs plugin update to current version 4.7.1
EdColeman opened a new pull request, #2824:
URL: https://github.com/apache/accumulo/pull/2824
Updating spotbugs to current release. This does not change the error check level
Fixes new error
```
GarbageCollectionLogger.java:[line 117] SSD_DO_NOT_USE_INSTANCE_LOCK_ON_SHARED_STATIC_DATA
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman merged pull request #2824: Spotbugs plugin update to current version 4.7.1
Posted by GitBox <gi...@apache.org>.
EdColeman merged PR #2824:
URL: https://github.com/apache/accumulo/pull/2824
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on pull request #2824: Spotbugs plugin update to current version 4.7.1
Posted by GitBox <gi...@apache.org>.
milleruntime commented on PR #2824:
URL: https://github.com/apache/accumulo/pull/2824#issuecomment-1194141665
> but seemed the best course of action was to preserve the current behavior
Do we want to preserve the current behavior though? It seems like spotbugs has a legitimate reason to not do what we are doing here.
> If there are multiple instances created in a server, the current behavior is that the log / check will run once per defined period even if there were multiple instance created.
I think the only way to have 2 GCLoggers running is if you have 2 tservers running in the same JVM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a diff in pull request #2824: Spotbugs plugin update to current version 4.7.1
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on code in PR #2824:
URL: https://github.com/apache/accumulo/pull/2824#discussion_r929289360
##########
pom.xml:
##########
@@ -720,7 +720,7 @@
<plugin>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
- <version>4.5.3.0</version>
+ <version>4.7.1.0</version>
Review Comment:
I had been working on getting us to 4.7.0.0. It generated a bunch of new warnings. If this passes, I'm wondering if they rolled some of those changes back a bit for 4.7.1.0, due to them being too aggressive?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on pull request #2824: Spotbugs plugin update to current version 4.7.1
Posted by GitBox <gi...@apache.org>.
EdColeman commented on PR #2824:
URL: https://github.com/apache/accumulo/pull/2824#issuecomment-1194107775
Making `lastMemoryCheckTime` private changes the current behavior. Not sure if that is necessary - but seemed the best course of action was to preserve the current behavior and then consider refactoring. If there are multiple instances created in a server, the current behavior is that the log / check will run once per defined period even if there were multiple instance created.
It may be desirable to lift the GCLogger to AbstractSever so that it is initialized / runs once per server process without static and possible synchronization required - but that seemed outside of the scope of this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a diff in pull request #2824: Spotbugs plugin update to current version 4.7.1
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on code in PR #2824:
URL: https://github.com/apache/accumulo/pull/2824#discussion_r929200619
##########
server/base/src/main/java/org/apache/accumulo/server/GarbageCollectionLogger.java:
##########
@@ -35,7 +36,7 @@ public class GarbageCollectionLogger {
private final HashMap<String,Long> prevGcTime = new HashMap<>();
private long lastMemorySize = 0;
private long gcTimeIncreasedCount = 0;
- private static long lastMemoryCheckTime = 0;
+ private static AtomicLong lastMemoryCheckTime = new AtomicLong(0);
Review Comment:
This should be final.
```suggestion
private static final AtomicLong lastMemoryCheckTime = new AtomicLong(0);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org