This is an automated email from the ASF dual-hosted git repository. chengpan pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/incubator-celeborn.git
commit eb382b018c35d87544d5a44b0126259323c06bff Author: Mridul Muralidharan <[email protected]> AuthorDate: Sun Oct 22 22:31:06 2023 +0800 [CELEBORN-1072] Fix misc error prone reports found Fix misc error prone reports. As detailed in the jira, they are: * Reference equality of boxed primitive types: see [BoxedPrimitiveEquality](https://errorprone.info/bugpattern/BoxedPrimitiveEquality) * Calling run directly - since use is legitimate, mark it as ignore. See: [DoNotCall](https://errorprone.info/bugpattern/DoNotCall) * `Ignore` test instead of catching `AssertionError` and ignoring it. See: [AssertionFailureIgnored](https://errorprone.info/bugpattern/AssertionFailureIgnored) Fix misc error prone reports. No Unit tests Closes #2019 from mridulm/fix-misc-issues. Authored-by: Mridul Muralidharan <mridulatgmail.com> Signed-off-by: zky.zhoukeyong <[email protected]> --- .../spark/shuffle/celeborn/PackedRecordPointerSuiteJ.java | 13 +++++-------- .../celeborn/common/network/protocol/RegionStart.java | 6 +++--- .../common/network/TransportClientFactorySuiteJ.java | 1 + .../celeborn/service/deploy/worker/PushDataHandler.scala | 4 ++-- 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/client-spark/common/src/test/java/org/apache/spark/shuffle/celeborn/PackedRecordPointerSuiteJ.java b/client-spark/common/src/test/java/org/apache/spark/shuffle/celeborn/PackedRecordPointerSuiteJ.java index 5a5c40459..53aa26921 100644 --- a/client-spark/common/src/test/java/org/apache/spark/shuffle/celeborn/PackedRecordPointerSuiteJ.java +++ b/client-spark/common/src/test/java/org/apache/spark/shuffle/celeborn/PackedRecordPointerSuiteJ.java @@ -25,6 +25,7 @@ import java.io.IOException; import org.apache.spark.SparkConf; import org.apache.spark.memory.*; import org.apache.spark.unsafe.memory.MemoryBlock; +import org.junit.Ignore; import org.junit.Test; public class PackedRecordPointerSuiteJ { @@ -80,16 +81,12 @@ public class PackedRecordPointerSuiteJ { assertEquals(MAXIMUM_PARTITION_ID, packedPointer.getPartitionId()); } - @Test + @Ignore public void partitionIdsGreaterThanMaximumPartitionIdWillOverflowOrTriggerError() { PackedRecordPointer packedPointer = new PackedRecordPointer(); - try { - // Pointers greater than the maximum partition ID will overflow or trigger an assertion error - packedPointer.set(PackedRecordPointer.packPointer(0, MAXIMUM_PARTITION_ID + 1)); - assertFalse(MAXIMUM_PARTITION_ID + 1 == packedPointer.getPartitionId()); - } catch (AssertionError e) { - // pass - } + // Pointers greater than the maximum partition ID will overflow or trigger an assertion error + packedPointer.set(PackedRecordPointer.packPointer(0, MAXIMUM_PARTITION_ID + 1)); + assertFalse(MAXIMUM_PARTITION_ID + 1 == packedPointer.getPartitionId()); } @Test diff --git a/common/src/main/java/org/apache/celeborn/common/network/protocol/RegionStart.java b/common/src/main/java/org/apache/celeborn/common/network/protocol/RegionStart.java index 4081c5cf4..2c327c880 100644 --- a/common/src/main/java/org/apache/celeborn/common/network/protocol/RegionStart.java +++ b/common/src/main/java/org/apache/celeborn/common/network/protocol/RegionStart.java @@ -29,8 +29,8 @@ public final class RegionStart extends RequestMessage { public final String shuffleKey; public final String partitionUniqueId; public final int attemptId; - public int currentRegionIndex; - public Boolean isBroadcast; + public final int currentRegionIndex; + public final boolean isBroadcast; public RegionStart( byte mode, @@ -38,7 +38,7 @@ public final class RegionStart extends RequestMessage { String partitionUniqueId, int attemptId, int currentRegionIndex, - Boolean isBroadcast) { + boolean isBroadcast) { this.mode = mode; this.shuffleKey = shuffleKey; this.partitionUniqueId = partitionUniqueId; diff --git a/common/src/test/java/org/apache/celeborn/common/network/TransportClientFactorySuiteJ.java b/common/src/test/java/org/apache/celeborn/common/network/TransportClientFactorySuiteJ.java index 89db68998..022af8178 100644 --- a/common/src/test/java/org/apache/celeborn/common/network/TransportClientFactorySuiteJ.java +++ b/common/src/test/java/org/apache/celeborn/common/network/TransportClientFactorySuiteJ.java @@ -62,6 +62,7 @@ public class TransportClientFactorySuiteJ { * * <p>If concurrent is true, create multiple threads to create clients in parallel. */ + @SuppressWarnings("DoNotCall") private void testClientReuse(int maxConnections, boolean concurrent) throws IOException, InterruptedException { diff --git a/worker/src/main/scala/org/apache/celeborn/service/deploy/worker/PushDataHandler.scala b/worker/src/main/scala/org/apache/celeborn/service/deploy/worker/PushDataHandler.scala index 62edc2a93..a1bf40f57 100644 --- a/worker/src/main/scala/org/apache/celeborn/service/deploy/worker/PushDataHandler.scala +++ b/worker/src/main/scala/org/apache/celeborn/service/deploy/worker/PushDataHandler.scala @@ -1043,7 +1043,7 @@ class PushDataHandler extends BaseMessageHandler with Logging { numPartitions, bufferSize) case Type.REGION_START => - val (currentRegionIndex, isBroadcast) = + val (currentRegionIndex, isBroadcast: Boolean) = if (isLegacy) ( msg.asInstanceOf[RegionStart].currentRegionIndex, @@ -1051,7 +1051,7 @@ class PushDataHandler extends BaseMessageHandler with Logging { else ( pbMsg.asInstanceOf[PbRegionStart].getCurrentRegionIndex, - Boolean.box(pbMsg.asInstanceOf[PbRegionStart].getIsBroadcast)) + pbMsg.asInstanceOf[PbRegionStart].getIsBroadcast) fileWriter.asInstanceOf[MapPartitionFileWriter].regionStart( currentRegionIndex, isBroadcast)
