Apache9 commented on a change in pull request #3936:
URL: https://github.com/apache/hbase/pull/3936#discussion_r767847725
##########
File path:
hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestCoprocessorDescriptor.java
##########
@@ -71,7 +67,11 @@ public void testSetCoprocessor() throws IOException {
List<CoprocessorDescriptor> cps = new ArrayList<>();
for (String className : Arrays.asList("className0", "className1",
"className2")) {
String path = "path";
- int priority = Math.abs(className.hashCode());
+ // Ensure priority is a positive integer
+ int priority = className.hashCode();
+ if (priority < 0) {
+ priority = -priority;
Review comment:
This is not the correct fix... You are just repeating the implementation
of Math.abs here...
If priority is Integer.MIN_VALUE, then -priority will still be
Integer.MIN_VALUE...
##########
File path:
hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestAvlUtil.java
##########
@@ -54,9 +55,8 @@ public void testAvlTreeCrud() {
final TreeMap<Integer, Object> treeMap = new TreeMap<>();
TestAvlNode root = null;
- final Random rand = new Random();
for (int i = 0; i < NELEM; ++i) {
- int key = rand.nextInt(MAX_KEY);
+ int key = RNG.nextInt(MAX_KEY);
Review comment:
Ditto, since we do not have a fixed seed here, let's just use
ThreadLocalRandom.
##########
File path:
hbase-it/src/test/java/org/apache/hadoop/hbase/mapreduce/IntegrationTestBulkLoad.java
##########
@@ -373,7 +375,11 @@ public void close() throws IOException {
taskId = taskId + iteration * numMapTasks;
numMapTasks = numMapTasks * numIterations;
- long chainId = Math.abs(new Random().nextLong());
+ // ensure chainId is positive
+ long chainId = RNG.nextLong();
+ if (chainId < 0) {
Review comment:
Ditto, not the correct way for fixing the abs problem...
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/PerClientRandomNonceGenerator.java
##########
@@ -32,13 +32,14 @@
public final class PerClientRandomNonceGenerator implements NonceGenerator {
private static final PerClientRandomNonceGenerator INST = new
PerClientRandomNonceGenerator();
-
- private final Random rdm = new Random();
private final long clientId;
+ private Random rng;
Review comment:
I do not see any real differences here? What is the spotbugs issue here?
##########
File path:
hbase-common/src/test/java/org/apache/hadoop/hbase/io/util/TestLRUDictionary.java
##########
@@ -81,9 +83,8 @@ public void testPassingSameArrayToAddEntry() {
@Test
public void testBasic() {
- Random rand = new Random();
byte[] testBytes = new byte[10];
- rand.nextBytes(testBytes);
+ RNG.nextBytes(testBytes);
Review comment:
What is the real gain here? We just the the random once, and I suggest
that we just use ThreadLocalRandom instead...
##########
File path:
hbase-it/src/test/java/org/apache/hadoop/hbase/TestIntegrationTestBase.java
##########
@@ -40,11 +40,10 @@ public void testMonkeyPropertiesParsing() {
conf.set(MonkeyConstants.BATCH_RESTART_RS_RATIO, "0.85");
conf.set(MonkeyConstants.MOVE_REGIONS_MAX_TIME, "60000");
conf.set("hbase.rootdir", "/foo/bar/baz");
-
final Properties props = new Properties();
- IntegrationTestBase testBase = new IntegrationTestDDLMasterFailover();
assertEquals(0, props.size());
- testBase.loadMonkeyProperties(props, conf);
+ new IntegrationTestDDLMasterFailover();
Review comment:
Why a new without assignment?
--
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: issues-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
