Merge pull request #50 from sirensolutions/FEDE-7220-arrow-upgrade-ch…

…eck-we-can-use-memory-unsafe-instead-of-memory-netty [FEDE-7220] arrow upgrade - check we can use memory unsafe instead of memory netty
sirensolutions · Sep 17, 2024 · 826becb · 826becb
2 parents a61f4af + 505495e
commit 826becb
Show file tree

Hide file tree

Showing 20 changed files with 149 additions and 44 deletions.
diff --git a/README.md b/README.md
@@ -17,6 +17,62 @@
   under the License.
 -->
 
+# Siren fork of Arrow
+
+The properties `drill.enable_unsafe_memory_access` and
+  `arrow.enable_unsafe_memory_access` are prefixed with `siren` and their
+  default value is set to `true`. The first property is deprecated.
+
+## Check that Arrow uses Unsafe class to access off-heap memory for memory allocation
+In order to check that Arrow uses Unsafe class for memory allocation, run the unit test `CheckArrowTest` in 
+  `https://github.com/sirensolutions/siren-platform/blob/master/core/src/test/java/io/siren/federate/core/common/CheckArrowTest.java`.
+
+## Build
+
+To build the `memory`, `format` and `vector` modules:
+
+```sh
+$ cd java
+$ mvn clean package
+```
+
+Because of the default value change of `unsafe_memory_access` property, some
+tests in `vector` fail.
+
+```sh
+mvn -pl maven,maven/module-info-compiler-maven-plugin,memory,memory/memory-core,memory/memory-unsafe,format,vector install -Dsiren.arrow.enable_unsafe_memory_access=false -Dsiren.drill.enable_unsafe_memory_access=false
+```
+
+## Make a new release of Siren's Apache Arrow
+
+- Tests should pass.
+
+- Make a new version:
+
+```sh
+mvn versions:set -DnewVersion=siren-0.14.1-2
+```
+
+- tag the commit for the release
+
+```sh
+git tag --sign siren-0.14.1-2
+````
+
+- Deploy to Siren's artifactory
+
+```sh
+$ mvn deploy -DskipTests=true -P artifactory -Dartifactory_username=<USERNAME> -Dartifactory_password=<PASSWORD>
+```
+
+## Update to a new version of Siren's Apache Arrow
+Developer tips on updating to a new version of Arrow can be found here: https://sirensolutions.atlassian.net/wiki/spaces/EN/pages/3108864001/Upgrading+Federate+Apache+Arrow+Version .
+
+- add `git@github.com:apache/arrow.git` as the `upstream` remote.
+- execute `git fetch --all --tags`
+- create a temporary branch from `siren-changes`
+- rebase against the new tag.
+
 # Apache Arrow
 
 [![Fuzzing Status](https://oss-fuzz-build-logs.storage.googleapis.com/badges/arrow.svg)](https://bugs.chromium.org/p/oss-fuzz/issues/list?sort=-opened&can=1&q=proj:arrow)

diff --git a/cpp/submodules/parquet-testing b/cpp/submodules/parquet-testing
diff --git a/java/adapter/orc/pom.xml b/java/adapter/orc/pom.xml
@@ -111,7 +111,7 @@
     <parent>
         <groupId>org.apache.arrow</groupId>
         <artifactId>arrow-java-root</artifactId>
-        <version>15.0.0</version>
+        <version>siren-15.0.0-2-SNAPSHOT</version>
         <relativePath>../../pom.xml</relativePath>
     </parent>
 

diff --git a/java/format/pom.xml b/java/format/pom.xml
@@ -15,7 +15,7 @@
 <parent>
   <artifactId>arrow-java-root</artifactId>
   <groupId>org.apache.arrow</groupId>
-  <version>15.0.0</version>
+  <version>siren-15.0.0-2-SNAPSHOT</version>
 </parent>
 
 <artifactId>arrow-format</artifactId>

diff --git a/java/gandiva/pom.xml b/java/gandiva/pom.xml
@@ -14,7 +14,7 @@
     <parent>
       <groupId>org.apache.arrow</groupId>
       <artifactId>arrow-java-root</artifactId>
-      <version>15.0.0</version>
+      <version>siren-15.0.0-2-SNAPSHOT</version>
     </parent>
 
     <groupId>org.apache.arrow.gandiva</groupId>

diff --git a/java/maven/module-info-compiler-maven-plugin/pom.xml b/java/maven/module-info-compiler-maven-plugin/pom.xml
@@ -16,7 +16,7 @@
   <parent>
     <groupId>org.apache.arrow.maven.plugins</groupId>
     <artifactId>arrow-maven-plugins</artifactId>
-    <version>15.0.0</version>
+    <version>siren-15.0.0-2-SNAPSHOT</version>
   </parent>
   <artifactId>module-info-compiler-maven-plugin</artifactId>
   <packaging>maven-plugin</packaging>
@@ -127,4 +127,4 @@
   </build>
 
 
-</project>
+</project>
diff --git a/java/maven/pom.xml b/java/maven/pom.xml
@@ -17,7 +17,7 @@
   -->
   <groupId>org.apache.arrow.maven.plugins</groupId>
   <artifactId>arrow-maven-plugins</artifactId>
-  <version>15.0.0</version>
+  <version>siren-15.0.0-2-SNAPSHOT</version>
   <name>Arrow Maven Plugins</name>
   <packaging>pom</packaging>
 
@@ -342,4 +342,25 @@
       </plugin>
     </plugins>
   </reporting>
+
+  <!-- To deploy to Siren artifactory -->
+  <profiles>
+    <profile>
+      <id>artifactory</id>
+
+      <distributionManagement>
+        <repository>
+          <id>artifactory-releases</id>
+          <name>artifactory-releases</name>
+          <url>${artifactory.url}/libs-release-local</url>
+        </repository>
+        <snapshotRepository>
+          <id>artifactory-snapshots</id>
+          <name>artifactory-snapshots</name>
+          <url>${artifactory.url}/libs-snapshot-local</url>
+        </snapshotRepository>
+      </distributionManagement>
+    </profile>
+  </profiles>
+
 </project>
diff --git a/java/memory/memory-core/pom.xml b/java/memory/memory-core/pom.xml
@@ -13,7 +13,7 @@
   <parent>
     <artifactId>arrow-memory</artifactId>
     <groupId>org.apache.arrow</groupId>
-    <version>15.0.0</version>
+    <version>siren-15.0.0-2-SNAPSHOT</version>
   </parent>
   <modelVersion>4.0.0</modelVersion>
 

diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BoundsChecking.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BoundsChecking.java
@@ -34,14 +34,15 @@ public class BoundsChecking {
   static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(BoundsChecking.class);
 
   static {
-    String envProperty = System.getenv("ARROW_ENABLE_UNSAFE_MEMORY_ACCESS");
-    String oldProperty = System.getProperty("drill.enable_unsafe_memory_access");
+    String envProperty = System.getenv().getOrDefault("SIREN_ARROW_ENABLE_UNSAFE_MEMORY_ACCESS", "true");
+    String oldProperty = System.getProperty("siren.drill.enable_unsafe_memory_access", "true");
     if (oldProperty != null) {
-      logger.warn("\"drill.enable_unsafe_memory_access\" has been renamed to \"arrow.enable_unsafe_memory_access\"");
-      logger.warn("\"arrow.enable_unsafe_memory_access\" can be set to: " +
-              " true (to not check) or false (to check, default)");
+      logger.warn("\"siren.drill.enable_unsafe_memory_access\" has been renamed to " +
+              "\"siren.arrow.enable_unsafe_memory_access\"");
+      logger.warn("\"siren.arrow.enable_unsafe_memory_access\" can be set to: " +
+              " true (to not check, default) or false (to check)");
     }
-    String newProperty = System.getProperty("arrow.enable_unsafe_memory_access");
+    String newProperty = System.getProperty("siren.arrow.enable_unsafe_memory_access", "true");
 
     // The priority of determining the unsafe flag:
     // 1. The system properties take precedence over the environmental variable.

diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java
@@ -85,7 +85,6 @@ public Object run() {
 
       // get the offset of the address field in a java.nio.Buffer object
       Field addressField = java.nio.Buffer.class.getDeclaredField("address");
-      addressField.setAccessible(true);
       BYTE_BUFFER_ADDRESS_OFFSET = UNSAFE.objectFieldOffset(addressField);
 
       Constructor<?> directBufferConstructor;
@@ -104,10 +103,7 @@ public Object run() {
                   constructor.setAccessible(true);
                   logger.debug("Constructor for direct buffer found and made accessible");
                   return constructor;
-                } catch (NoSuchMethodException e) {
-                  logger.debug("Cannot get constructor for direct buffer allocation", e);
-                  return e;
-                } catch (SecurityException e) {
+                } catch (Exception e) {
                   logger.debug("Cannot get constructor for direct buffer allocation", e);
                   return e;
                 }

diff --git a/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBoundaryChecking.java b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBoundaryChecking.java
@@ -55,15 +55,15 @@ private boolean getFlagValue(ClassLoader classLoader) throws Exception {
   }
 
   /**
-   * Ensure the flag for bounds checking is enabled by default.
-   * This will protect users from JVM crashes.
+   * Siren: Ensure the flag for bounds checking is disabled by default.
+   * Enabling it will protect users from JVM crashes.
    */
   @Test
   public void testDefaultValue() throws Exception {
     ClassLoader classLoader = copyClassLoader();
     if (classLoader != null) {
       boolean boundsCheckingEnabled = getFlagValue(classLoader);
-      Assert.assertTrue(boundsCheckingEnabled);
+      Assert.assertFalse(boundsCheckingEnabled);
     }
   }
 

diff --git a/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestOpens.java b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestOpens.java
@@ -20,11 +20,13 @@
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import org.junit.Ignore;
 import org.junit.Test;
 
 public class TestOpens {
   /** Instantiating the RootAllocator should poke MemoryUtil and fail. */
   @Test
+  @Ignore
   public void testMemoryUtilFailsLoudly() {
     // This test is configured by Maven to run WITHOUT add-opens. So this should fail on JDK16+
     // (where JEP396 means that add-opens is required to access JDK internals).

diff --git a/java/memory/memory-netty/pom.xml b/java/memory/memory-netty/pom.xml
@@ -13,7 +13,7 @@
   <parent>
     <artifactId>arrow-memory</artifactId>
     <groupId>org.apache.arrow</groupId>
-    <version>15.0.0</version>
+    <version>siren-15.0.0-2-SNAPSHOT</version>
   </parent>
   <modelVersion>4.0.0</modelVersion>
 
@@ -34,9 +34,11 @@
       <groupId>io.netty</groupId>
       <artifactId>netty-common</artifactId>
     </dependency>
+    <!--provided by elasticsearch-->
     <dependency>
       <groupId>org.slf4j</groupId>
       <artifactId>slf4j-api</artifactId>
+      <scope>provided</scope>
     </dependency>
     <dependency>
       <groupId>ch.qos.logback</groupId>

diff --git a/java/memory/memory-unsafe/pom.xml b/java/memory/memory-unsafe/pom.xml
@@ -13,7 +13,7 @@
   <parent>
     <artifactId>arrow-memory</artifactId>
     <groupId>org.apache.arrow</groupId>
-    <version>15.0.0</version>
+    <version>siren-15.0.0-2-SNAPSHOT</version>
   </parent>
   <modelVersion>4.0.0</modelVersion>
 

diff --git a/java/memory/pom.xml b/java/memory/pom.xml
@@ -14,7 +14,7 @@
   <parent>
     <groupId>org.apache.arrow</groupId>
     <artifactId>arrow-java-root</artifactId>
-    <version>15.0.0</version>
+    <version>siren-15.0.0-2-SNAPSHOT</version>
   </parent>
   <artifactId>arrow-memory</artifactId>
   <name>Arrow Memory</name>

diff --git a/java/pom.xml b/java/pom.xml
@@ -20,7 +20,7 @@
 
   <groupId>org.apache.arrow</groupId>
   <artifactId>arrow-java-root</artifactId>
-  <version>15.0.0</version>
+  <version>siren-15.0.0-2-SNAPSHOT</version>
   <packaging>pom</packaging>
 
   <name>Apache Arrow Java Root POM</name>
@@ -349,7 +349,7 @@
             </goals>
             <configuration>
               <ignoreNonCompile>true</ignoreNonCompile>
-              <failOnWarning>true</failOnWarning>
+              <failOnWarning>false</failOnWarning>
               <ignoredDependencies>
                 <!-- source annotations (not kept in compiled code) -->
                 <ignoredDependency>javax.annotation:javax.annotation-api:*</ignoredDependency>
@@ -602,7 +602,7 @@
       <dependency>
         <groupId>org.apache.arrow</groupId>
         <artifactId>arrow-bom</artifactId>
-        <version>${project.version}</version>
+        <version>15.0.0</version>
         <type>pom</type>
         <scope>import</scope>
       </dependency>
@@ -792,17 +792,18 @@
 
   <modules>
     <module>maven</module>
-    <module>bom</module>
+    <!--<module>bom</module>-->
     <module>format</module>
     <module>memory</module>
     <module>vector</module>
-    <module>tools</module>
-    <module>adapter/jdbc</module>
-    <module>flight</module>
-    <module>performance</module>
-    <module>algorithm</module>
-    <module>adapter/avro</module>
-    <module>compression</module>
+    <!--<module>tools</module>-->
+    <!--<module>adapter/jdbc</module>-->
+    <!--<module>plasma</module>-->
+    <!--<module>flight</module>-->
+    <!--<module>performance</module>-->
+    <!--<module>algorithm</module>-->
+    <!--<module>adapter/avro</module>-->
+    <!--<module>compression</module>-->
   </modules>
 
   <profiles>
@@ -1293,6 +1294,25 @@
         </plugins>
       </build>
     </profile>
+
+    <!-- To deploy to Siren artifactory -->
+    <profile>
+      <id>artifactory</id>
+
+      <distributionManagement>
+        <repository>
+          <id>artifactory-releases</id>
+          <name>artifactory-releases</name>
+          <url>${artifactory.url}/libs-release-local</url>
+        </repository>
+        <snapshotRepository>
+          <id>artifactory-snapshots</id>
+          <name>artifactory-snapshots</name>
+          <url>${artifactory.url}/libs-snapshot-local</url>
+        </snapshotRepository>
+      </distributionManagement>
+    </profile>
+
   </profiles>
 
 </project>