subject:"\[GitHub\] \[arrow\] BryanCutler commented on a change in pull request #7619\: ARROW\-9300\: \[Java\] Separate Netty Memory to its own module"

[GitHub] [arrow] BryanCutler commented on a change in pull request #7619: ARROW-9300: [Java] Separate Netty Memory to its own module

2020-07-09 Thread GitBox



BryanCutler commented on a change in pull request #7619:
URL: https://github.com/apache/arrow/pull/7619#discussion_r452341314



##
File path: 
java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java
##
@@ -114,10 +114,20 @@ static AllocationManagerType 
getDefaultAllocationManagerType() {
   }
 
   private static AllocationManager.Factory getUnsafeFactory() {
-return getFactory("org.apache.arrow.memory.UnsafeAllocationManager");
+try {
+  return getFactory("org.apache.arrow.memory.UnsafeAllocationManager");
+} catch (RuntimeException e) {
+  throw new RuntimeException("Please add arrow-memory-unsafe to your 
classpath," +
+  " No DefaultAllocationManager found to instantiate an 
UnsafeAllocationManager", e);
+}
   }
 
   private static AllocationManager.Factory getNettyFactory() {
-return getFactory("org.apache.arrow.memory.NettyAllocationManager");
+try {
+  return getFactory("org.apache.arrow.memory.NettyAllocationManager");
+} catch (RuntimeException e) {
+  throw new RuntimeException("Please add arrow-memory-unsafe to your 
classpath," +

Review comment:
   should be `arrow-memory-netty` and `NettyAllocationManager` below





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [arrow] BryanCutler commented on a change in pull request #7619: ARROW-9300: [Java] Separate Netty Memory to its own module

2020-07-08 Thread GitBox



BryanCutler commented on a change in pull request #7619:
URL: https://github.com/apache/arrow/pull/7619#discussion_r451797963



##
File path: 
java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java
##
@@ -109,7 +109,8 @@ static AllocationManagerType 
getDefaultAllocationManagerType() {
   field.setAccessible(true);
   return (AllocationManager.Factory) field.get(null);
 } catch (Exception e) {
-  throw new RuntimeException("Unable to instantiate Allocation Manager for 
" + clazzName, e);
+  throw new RuntimeException("No DefaultAllocationManager found on 
classpath. Can't allocate Arrow buffers." +
+  " Please consider adding arrow-memory-netty or arrow-memory-unsafe 
as a dependency.");

Review comment:
   Hmm, this one is a little different. It looks like it could be from 
`getUnsafeFactory()`, `getNettyFactory()`, or some custom class.  I would leave 
the original exception with `clazzName`, then in `getUnsafeFactory()`, 
`getNettyFactory()` catch the RuntimeException and add a message like "Please 
ensure [arrow-memory-netty, arrow-memory-unsafe] or equivalent class containing 
the DefaultAllocationManager is on the classpath"





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [arrow] BryanCutler commented on a change in pull request #7619: ARROW-9300: [Java] Separate Netty Memory to its own module

2020-07-08 Thread GitBox



BryanCutler commented on a change in pull request #7619:
URL: https://github.com/apache/arrow/pull/7619#discussion_r451792885



##
File path: java/memory/memory-unsafe/pom.xml
##
@@ -0,0 +1,54 @@
+
+
+http://maven.apache.org/POM/4.0.0;
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
+  
+arrow-memory
+org.apache.arrow
+1.0.0-SNAPSHOT
+  
+  4.0.0
+
+  arrow-memory-unsafe
+  Arrow Memory - Unsafe
+  Allocator and utils for allocating memory in Arrow based on 
sun.misc.Unsafe
+
+
+  
+
+  org.apache.arrow
+  arrow-memory-core
+  ${project.version}
+
+  
+
+  
+
+  
+maven-surefire-plugin
+3.0.0-M3
+
+  true
+  true
+  ${forkCount}
+  true
+  
+${project.build.directory}
+UTC
+  
+
+  
+
+  
+
+

Review comment:
   It's usually conventional to have it at the end of text files and some 
command line tools expect it. Not sure why it's not caught by the style 
checker, but that's why github marks it with a red icon.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [arrow] BryanCutler commented on a change in pull request #7619: ARROW-9300: [Java] Separate Netty Memory to its own module

2020-07-08 Thread GitBox



BryanCutler commented on a change in pull request #7619:
URL: https://github.com/apache/arrow/pull/7619#discussion_r451710176



##
File path: java/memory/memory-netty/pom.xml
##
@@ -0,0 +1,107 @@
+
+
+http://maven.apache.org/POM/4.0.0;
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
+  
+arrow-memory
+org.apache.arrow
+1.0.0-SNAPSHOT
+  
+  4.0.0
+
+  arrow-memory-netty
+  Arrow Memory - Netty
+  Netty allocator and utils for allocating memory in 
Arrow
+
+  
+
+  org.apache.arrow
+  arrow-memory-core
+  ${project.version}
+
+
+  io.netty
+  netty-buffer
+
+
+  io.netty
+  netty-common
+
+
+  org.slf4j
+  slf4j-api
+
+
+  org.immutables
+  value
+
+  
+
+  
+
+  
+maven-surefire-plugin
+3.0.0-M3
+
+  true
+  true
+  ${forkCount}
+  true
+  
+${project.build.directory}
+
true
+
1048576
+UTC
+  
+  
+  -Darrow.vector.max_allocation_bytes=1048576
+
+  
+
+  
+
+  
+
+  
+  integration-tests
+  
+
+  
+org.apache.maven.plugins
+maven-failsafe-plugin
+
+  
+${project.build.directory}
+
true
+UTC
+  
+  
+
+
+  
+
+  integration-test
+  verify
+
+  
+
+  
+
+  
+
+  
+
+

Review comment:
   it still looks like it needs a newline at the end of the file

##
File path: java/memory/memory-unsafe/pom.xml
##
@@ -0,0 +1,54 @@
+
+
+http://maven.apache.org/POM/4.0.0;
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
+  
+arrow-memory
+org.apache.arrow
+1.0.0-SNAPSHOT
+  
+  4.0.0
+
+  arrow-memory-unsafe
+  Arrow Memory - Unsafe
+  Allocator and utils for allocating memory in Arrow based on 
sun.misc.Unsafe
+
+
+  
+
+  org.apache.arrow
+  arrow-memory-core
+  ${project.version}
+
+  
+
+  
+
+  
+maven-surefire-plugin
+3.0.0-M3
+
+  true
+  true
+  ${forkCount}
+  true
+  
+${project.build.directory}
+UTC
+  
+
+  
+
+  
+
+

Review comment:
   newline here

##
File path: java/memory/memory-core/pom.xml
##
@@ -0,0 +1,60 @@
+
+
+http://maven.apache.org/POM/4.0.0;
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
+  
+arrow-memory
+org.apache.arrow
+1.0.0-SNAPSHOT
+  
+  4.0.0
+
+  arrow-memory-core
+
+  Arrow Memory - Core
+  Core off-heap memory management libraries for Arrow 
ValueVectors.
+
+  
+
+  com.google.code.findbugs
+  jsr305
+
+
+  org.slf4j
+  slf4j-api
+
+
+  org.immutables
+  value
+
+  
+
+  
+
+  
+maven-surefire-plugin
+3.0.0-M3
+
+  true
+  true
+  ${forkCount}
+  true
+  
+${project.build.directory}
+UTC
+  
+
+  
+
+  
+

Review comment:
   newline here





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [arrow] BryanCutler commented on a change in pull request #7619: ARROW-9300: [Java] Separate Netty Memory to its own module

2020-07-07 Thread GitBox



BryanCutler commented on a change in pull request #7619:
URL: https://github.com/apache/arrow/pull/7619#discussion_r451102944



##
File path: java/adapter/orc/pom.xml
##
@@ -15,10 +15,16 @@
 
 
 org.apache.arrow
-arrow-memory
-${project.version}
+arrow-memory-core
+1.0.0-SNAPSHOT

Review comment:
   was this intentional?

##
File path: java/memory/memory-core/pom.xml
##
@@ -0,0 +1,65 @@
+
+
+http://maven.apache.org/POM/4.0.0;
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
+  
+arrow-memory
+org.apache.arrow
+1.0.0-SNAPSHOT
+  
+  4.0.0
+
+  arrow-memory-core
+
+  Arrow Memory - Core
+  Core off-heap memory management libraries for Arrow 
ValueVectors.
+
+  
+
+  com.google.code.findbugs
+  jsr305
+
+
+  org.slf4j
+  slf4j-api
+
+
+  org.immutables
+  value
+
+  
+
+  
+
+  
+maven-surefire-plugin
+3.0.0-M3
+
+  true
+  true
+  ${forkCount}
+  true
+  
+${project.build.directory}
+
true

Review comment:
   is this necessary here?

##
File path: java/vector/pom.xml
##
@@ -50,6 +50,12 @@
   commons-codec
   1.10
 
+
+  org.apache.arrow
+  arrow-memory-netty
+  ${project.version}
+  test
+

Review comment:
   Maybe we should run the unit tests once for each of the 2 allocators?

##
File path: java/vector/pom.xml
##
@@ -50,6 +50,12 @@
   commons-codec
   1.10
 
+
+  org.apache.arrow
+  arrow-memory-netty
+  ${project.version}
+  test
+
 
   io.netty
   netty-buffer

Review comment:
   This could be removed now right?

##
File path: java/memory/memory-netty/pom.xml
##
@@ -0,0 +1,107 @@
+
+
+http://maven.apache.org/POM/4.0.0;
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
+  
+arrow-memory
+org.apache.arrow
+1.0.0-SNAPSHOT
+  
+  4.0.0
+
+  arrow-memory-netty
+  Arrow Memory - Netty
+  Netty allocator and utils for allocating memory in 
Arrow
+
+  
+
+  org.apache.arrow
+  arrow-memory-core
+  ${project.version}
+
+
+  io.netty
+  netty-buffer
+
+
+  io.netty
+  netty-common
+
+
+  org.slf4j
+  slf4j-api
+
+
+  org.immutables
+  value
+
+  
+
+  
+
+  
+maven-surefire-plugin
+3.0.0-M3
+
+  true
+  true
+  ${forkCount}
+  true
+  
+${project.build.directory}
+
true
+
1048576
+UTC
+  
+  
+  -Darrow.vector.max_allocation_bytes=1048576
+
+  
+
+  
+
+  
+
+  
+  integration-tests
+  
+
+  
+org.apache.maven.plugins
+maven-failsafe-plugin
+
+  
+${project.build.directory}
+
true
+UTC
+  
+  
+
+
+  
+
+  integration-test
+  verify
+
+  
+
+  
+
+  
+
+  
+
+

Review comment:
   add newline?





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [arrow] BryanCutler commented on a change in pull request #7619: ARROW-9300: [Java] Separate Netty Memory to its own module

[GitHub] [arrow] BryanCutler commented on a change in pull request #7619: ARROW-9300: [Java] Separate Netty Memory to its own module

[GitHub] [arrow] BryanCutler commented on a change in pull request #7619: ARROW-9300: [Java] Separate Netty Memory to its own module

[GitHub] [arrow] BryanCutler commented on a change in pull request #7619: ARROW-9300: [Java] Separate Netty Memory to its own module

[GitHub] [arrow] BryanCutler commented on a change in pull request #7619: ARROW-9300: [Java] Separate Netty Memory to its own module

5 matches

Site Navigation

Mail list logo

Footer information