Apache9 commented on code in PR #4645:
URL: https://github.com/apache/hbase/pull/4645#discussion_r950834395


##########
hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestByteBufferArray.java:
##########
@@ -119,13 +122,16 @@ public void testArrayIO() throws IOException {
     testReadAndWrite(array, cap - 2, 2, (byte) 10);
 
     expectedAssert(() -> testReadAndWrite(array, cap - 2, 3, (byte) 11));
-    expectedAssert(() -> testReadAndWrite(array, cap + 1, 0, (byte) 12));
     expectedAssert(() -> testReadAndWrite(array, 0, cap + 1, (byte) 12));
-    expectedAssert(() -> testReadAndWrite(array, -1, 0, (byte) 13));
     expectedAssert(() -> testReadAndWrite(array, 0, -23, (byte) 14));
-    expectedAssert(() -> testReadAndWrite(array, 0, 0, (byte) 15));
     expectedAssert(() -> testReadAndWrite(array, 4096, cap - 4096 + 1, (byte) 
16));
 
+    // XXX: These cases were apparently expected to assert but 
expectedAssert() was

Review Comment:
   Oops.



##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockType.java:
##########
@@ -31,6 +31,7 @@
  * The values in the enum appear in the order they appear in a version 2 HFile.
  */
 @InterfaceAudience.Private
+@SuppressWarnings("ImmutableEnumChecker")

Review Comment:
   I guess it is because the 'magic' field. It is a byte array, the content can 
be changed at runtime.



##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/BufferedDataBlockEncoder.java:
##########
@@ -279,14 +278,14 @@ private Cell toOffheapCell(ByteBuffer valAndTagsBuffer, 
int vOffset,
   protected static class OnheapDecodedCell implements ExtendedCell {
     private static final long FIXED_OVERHEAD = ClassSize.align(ClassSize.OBJECT
       + (3 * ClassSize.REFERENCE) + (2 * Bytes.SIZEOF_LONG) + (7 * 
Bytes.SIZEOF_INT)
-      + (Bytes.SIZEOF_SHORT) + (2 * Bytes.SIZEOF_BYTE) + (3 * 
ClassSize.ARRAY));
+      + Bytes.SIZEOF_SHORT + (2 * Bytes.SIZEOF_BYTE) + (3 * ClassSize.ARRAY));
     private byte[] keyOnlyBuffer;
     private short rowLength;
     private int familyOffset;
     private byte familyLength;
     private int qualifierOffset;
     private int qualifierLength;
-    private long timestamp;
+    private long timeStamp;

Review Comment:
   If we do not change the public method declaration I think it is fine. Can 
discuss this later.



##########
hbase-common/src/test/java/org/apache/hadoop/hbase/TestCellComparator.java:
##########
@@ -234,13 +228,10 @@ public void testBinaryKeys() throws Exception {
     // This will output the keys incorrectly.
     boolean assertion = false;
     int count = 0;
-    try {
-      for (Cell k : set) {
-        assertTrue("count=" + count + ", " + k.toString(), count++ == 
k.getTimestamp());
+    for (Cell k : set) {

Review Comment:
   Could try to use assertThrows in the future. Not a blocker.



##########
hbase-common/src/main/java/org/apache/hadoop/hbase/util/JVM.java:
##########
@@ -167,11 +170,10 @@ public long getOpenFileDescriptorCount() {
       // need to get the PID number of the process first
       RuntimeMXBean rtmbean = ManagementFactory.getRuntimeMXBean();
       String rtname = rtmbean.getName();
-      String[] pidhost = rtname.split("@");
-
+      Iterator<String> pidhost = Splitter.on('@').split(rtname).iterator();
       // using linux bash commands to retrieve info
       Process p = Runtime.getRuntime()
-        .exec(new String[] { "bash", "-c", "ls /proc/" + pidhost[0] + "/fdinfo 
| wc -l" });
+        .exec(new String[] { "bash", "-c", "ls /proc/" + pidhost.next() + 
"/fdinfo | wc -l" });

Review Comment:
   This could be done as a separated issue, not an error prone related problem.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to