[FEDE-7220] arrow upgrade - check we can use memory unsafe instead of memory netty #50

prateeknima77 · 2024-04-03T13:04:38Z

No description provided.

* [FEDE-5623] Upgrade Siren Arrow from 7.0.0 to 9.0.0 (#35) * [FEDE-5150] Upgrade arrow version from 4.0.0 to 7.0.0 (#26) * [FEDE-4869] Upgrade Arrow version to 4.0.0 (#16) * Make BaseValueVector#MAX_ALLOCATION_SIZE configurable This closes apache#65 Some of the tests are based on the assumption that the JVM can allocate at least 2GB of memory, which is not a common occurence (JVM usually defaults at 512MB). Current Travis CI VM only have 3GB of memory total, which would have make challenging to run some of the tests on them Add a system property to change BaseValueVector.MAX_ALLOCATION_SIZE to allow to use a much smaller value during tests. * prefix arrow's version with siren * use our version of netty * updated readme about siren's changes * fixed dependency issue with our own artifactory * use our version of netty * shade the arrow memory jar * improved doc * fix readme * document the changes done to the arrow fork * ARROW-5856: [Python] [Packaging] Fix use of C++ / Cython API from wheels Author: Antoine Pitrou <[email protected]> Closes apache#4884 from pitrou/ARROW-5856-cython-so-version and squashes the following commits: a411d7a <Antoine Pitrou> Avoid C++ ABI issues with DictionaryMemo eaede5b <Antoine Pitrou> Revert ARROW-5082 (" Stop exporting copies of shared libraries in wheel") 4594f78 <Antoine Pitrou> ARROW-5856: Try to fix Cython API from wheels * use our version of netty * set drill's default value * use our version of netty * bumped to 0.8.0 * update to 0.14.1 * comment unneeded modules * update release procedure with unneeded modules commented out * bump version to siren-0.14.1-1 and update readme * do not allocate a new offset buffer if the slice starts at 0 since the relative offset pointer would be unchanged * fix slice bounds * improved readme * Upgraded netty dependendcy to siren-4.1.27-3 * [FEDE-3917] netty direct memory counter deprecation with bump to siren-0.14.1-6-SNAPSHOT (#10) * Release siren-0.14.1-5 * Bump to version siren-0.14.1-6-SNAPSHOT * Use Siren Netty fork version siren-4.1.27-4 to release siren-0.14.1-6 * Bump version to release siren-0.14.1-5 using Netty Siren siren-4.1.27-4 * Set the version to siren-0.14.1-6-SNAPSHOT Co-authored-by: Martin Anseaume <[email protected]> * Fix rebase * Fix pom * Fix rebase - Bump version to siren-4.0.0-1-SNAPSHOT * Clean-up * Comment out memory-netty module and remove use siren netty * Update siren netty version * Fix rebase * Comment netty * Fix TestValueVector * Remove unused imports * Remove unused import * Clean up * Clean up - fix checkstyle * Clean up - fix checkstyle 2 * WIP * WIP - add siren netty plugin * WIP - ignore failing test * Fix rebase as per peer review * Fix rebase python * Fix ZeroVector unused imports * Fix checkstyle * Fix rebase * Fix rebase * Fix checkstyle imports violation * Fix checkstyle and update netty version * Use Arrow's ArrowBuf instead of our own * Fix imports * Fix checkstyle * Tentative fix: workaround for the shading issue but creat new problems * fix missing import * Fix import * Fix imports * Revert ignored unit tests * Revert automatic format changes * Revert format to original format * Update vector pom as per peer review * Revert ignored TestArrowBufHasher.testHasherNegative * Clean up * Update as per peer review * Undo unnecessary change in tasks.yml * Undo unnecessary changes * Revert changes * Revert changes * Revert changes as per peer review * Update as per peer review * Add back class path dependency exclusion * Remove whitespace * Update memory access with default value * Remove class path exclusion * Update readme * Exclude memory-core from the shaded netty in memory-netty package * Add information for checking that Siren version of Netty is used * [FEDE-5144] Fix the static initialization of MemoryUtil (#17) * Remove unecessary setAccessible call * Catch all errors thrown by the setAccessible call New version of java can throw InaccessibleObjectException which Arrow didn't handle * Update netty version to the stable version siren-4.1.48-1 Co-authored-by: Laurent Goujon <[email protected]> Co-authored-by: Stéphane Campinas <[email protected]> Co-authored-by: Antoine Pitrou <[email protected]> Co-authored-by: Martin Anseaume <[email protected]> Co-authored-by: Johnny Hujol <[email protected]> Co-authored-by: ggdupont <[email protected]> * Bumped version number to siren-4.0.0-1 * Fix rebase * Remove scope from arrow-memory-netty to make netty available for entire project else our shading will fail. Signed-off-by: George Apaaboah <[email protected]> * Fix readme - remove duplicate section on how to contribute * Update readme as per review * Use stable netty version siren-4.1.68-1 after the release of netty Signed-off-by: George Apaaboah <[email protected]> Co-authored-by: Laurent Goujon <[email protected]> Co-authored-by: Stéphane Campinas <[email protected]> Co-authored-by: Antoine Pitrou <[email protected]> Co-authored-by: Martin Anseaume <[email protected]> Co-authored-by: Johnny Hujol <[email protected]> Co-authored-by: ggdupont <[email protected]> * Release siren-7.0.0-1 Signed-off-by: George Apaaboah <[email protected]> * Fix rebase * Update services configuration (#32) * Update services configuration This is needed for the module declaration in federate * Create a uber jar with netty and arrow memory dependencies This avoids conflicts with java modules. Arrows provides different modules (memory-netty, memory-core) that export the same packages (org.apache.arrow.memory), which is forbidden with java modules. The federate code can then just dependend on the shaded jar from vector. * use reduced pom in jar Co-authored-by: Issac <[email protected]> * Add developer tip link to readme * Remove test scope for arrow-memory-netty * Change version from siren-9.0.0-1-SNAPSHOT to siren-9.0.0-2-SNAPSHOT since 2 contain the module changes * Update as per review * Update comment as per review * Use stable netty version siren-4.1.78-1 Signed-off-by: George Apaaboah <[email protected]> Co-authored-by: Laurent Goujon <[email protected]> Co-authored-by: Stéphane Campinas <[email protected]> Co-authored-by: Antoine Pitrou <[email protected]> Co-authored-by: Martin Anseaume <[email protected]> Co-authored-by: Johnny Hujol <[email protected]> Co-authored-by: ggdupont <[email protected]> Co-authored-by: Issac <[email protected]> * Fix cherry-pick * Temporarily comment out the jdk version * chore: revert commented out jdk version in pom * update: use stable version of netty after release of siren-4.1.82-1 --------- Signed-off-by: George Apaaboah <[email protected]> Co-authored-by: Laurent Goujon <[email protected]> Co-authored-by: Stéphane Campinas <[email protected]> Co-authored-by: Antoine Pitrou <[email protected]> Co-authored-by: Martin Anseaume <[email protected]> Co-authored-by: Johnny Hujol <[email protected]> Co-authored-by: ggdupont <[email protected]> Co-authored-by: Issac <[email protected]>

…nstall

github-actions · 2024-04-03T13:05:02Z

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

In the case of PARQUET issues on JIRA the title also supports:

PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

See also:

java/vector/pom.xml

GeorgeAp

Can you please remove extra spaces or added them back where it applies so that the fork only applies what we want to add?

java/vector/pom.xml

java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java

java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestOpens.java

prateeknima77 · 2024-04-23T17:35:16Z

Can you please remove extra spaces or added them back where it applies so that the fork only applies what we want to add?

Yes, the spacing has been adjusted in 1c369e8

java/memory/memory-netty/pom.xml

scampi

LGTM, just a clarification needed on a POM change

scampi · 2024-09-02T08:28:59Z

java/pom.xml

+            <!--<goals>
              <goal>analyze-only</goal>
-            </goals>
+            </goals>-->


why is this goal commented out ? Does it fail something ?

@scampi yes the changes were made in this commit and it occurs as analyze-only does stricter dependency analysis. It complains due to the following

[WARNING] Unused declared dependencies found: [WARNING] org.apache.arrow:arrow-memory-netty:jar:15.0.0:test [WARNING] org.apache.arrow:arrow-memory-unsafe:jar:siren-15.0.0-2-SNAPSHOT:compile [WARNING] org.slf4j:jul-to-slf4j:jar:2.0.9:test [WARNING] org.slf4j:jcl-over-slf4j:jar:2.0.9:test [WARNING] org.slf4j:log4j-over-slf4j:jar:2.0.9:test [WARNING] org.junit.jupiter:junit-jupiter-engine:jar:5.10.1:test [WARNING] org.junit.vintage:junit-vintage-engine:jar:5.10.1:test [WARNING] org.junit.jupiter:junit-jupiter-params:jar:5.10.1:test [WARNING] org.mockito:mockito-junit-jupiter:jar:2.25.1:test [WARNING] ch.qos.logback:logback-classic:jar:1.3.14:test [WARNING] de.huxhorn.lilith:de.huxhorn.lilith.logback.appender.multiplex-classic:jar:0.9.44:test

An alternative to get rid of this error is by changing <failOnWarning>true</failOnWarning> to false or do we investigate each of the dependencies further?

hmm it feels less intrusive to change the value of the failOnWarning parameter

Yes, I have uncommented the analyze-only goal and turned failOnWarning to false - 505495e and I have re-ran the other pr.

GeorgeAp

Can you please add a statement in the readme that from siren-15.0.x siren-netty is not used anymore ?

java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java

GeorgeAp and others added 20 commits February 23, 2024 14:41

update Poms

992695b

update: bump siren arrow to next version siren-11.0.0-3-SNAPSHOT (#47)

282d889

[FEDE-6850] feat: update netty to version siren-4.1.82-2 (#48)

7b5c6cd

bump version number to siren-11.0.0-4-SNAPSHOT

42b6d04

update: poms

5a9dff1

update: snapshot version

76010ad

chore: remove unused imports

77b48e9

chore: update readme

adfeb46

update: netty version

c9f0cc0

update: java bom

51d8de6

update: snapshot version

33c9657

chore: update readme

a83092b

update: discard project version from dependency

cff1c73

update: delete testing and parquet testing

8cd59c2

update: add project version

e1c6c8d

update: add Siren artifactory profile

be640a4

revert: parent artifact version

3c8bcca

update: comment out analyze-only goal - causing issues during maven i…

8fe1264

…nstall

update: Pom files to remove dependency on netty

455a949

github-actions bot added Component: Java Component: Documentation Component: C++ labels Apr 3, 2024

prateeknima77 added 2 commits April 18, 2024 09:08

update: remove shading

d0dd1ef

update: pom - discard exclusion of memory-unsafe

4e5f3e4

prateeknima77 marked this pull request as ready for review April 18, 2024 08:15

prateeknima77 requested review from GeorgeAp and torito April 18, 2024 08:16

prateeknima77 commented Apr 18, 2024

View reviewed changes

java/vector/pom.xml Outdated Show resolved Hide resolved

update: readme

01ad286

GeorgeAp requested changes Apr 18, 2024

View reviewed changes

java/vector/pom.xml Outdated Show resolved Hide resolved

java/vector/pom.xml Outdated Show resolved Hide resolved

java/vector/pom.xml Outdated Show resolved Hide resolved

java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java Show resolved Hide resolved

update: vector pom

fb7d427

GeorgeAp requested a review from scampi April 23, 2024 08:59

torito reviewed Apr 23, 2024

View reviewed changes

java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestOpens.java Show resolved Hide resolved

prateeknima77 added 2 commits April 23, 2024 17:58

update: revert vector pom changes

9a6b412

chore: remove extra spaces

1c369e8

prateeknima77 added 3 commits April 29, 2024 08:57

update: readme

a9b84cf

update: vector pom to add memory netty dependency

4a78ab5

fix: shadow memory-netty

dd3c51d

prateeknima77 commented Jul 30, 2024

View reviewed changes

java/memory/memory-netty/pom.xml Outdated Show resolved Hide resolved

prateeknima77 added 2 commits August 6, 2024 17:26

update: revert changes - discard addition of memory-netty

ce9ab8c

update:- snapshot version

21655f9

scampi approved these changes Sep 2, 2024

View reviewed changes

update: analyze-only goal - turn warnings check off

505495e

prateeknima77 requested review from GeorgeAp and torito September 5, 2024 10:37

GeorgeAp approved these changes Sep 5, 2024

View reviewed changes

GeorgeAp reviewed Sep 5, 2024

View reviewed changes

java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java Show resolved Hide resolved

torito approved these changes Sep 5, 2024

View reviewed changes

prateeknima77 merged commit 826becb into branch-siren-15.0.x Sep 17, 2024
13 of 54 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FEDE-7220] arrow upgrade - check we can use memory unsafe instead of memory netty #50

[FEDE-7220] arrow upgrade - check we can use memory unsafe instead of memory netty #50

prateeknima77 commented Apr 3, 2024

github-actions bot commented Apr 3, 2024

GeorgeAp left a comment

prateeknima77 commented Apr 23, 2024

scampi left a comment

scampi Sep 2, 2024

prateeknima77 Sep 2, 2024 •

edited

Loading

scampi Sep 5, 2024

prateeknima77 Sep 5, 2024

GeorgeAp left a comment

[FEDE-7220] arrow upgrade - check we can use memory unsafe instead of memory netty #50

[FEDE-7220] arrow upgrade - check we can use memory unsafe instead of memory netty #50

Conversation

prateeknima77 commented Apr 3, 2024

github-actions bot commented Apr 3, 2024

GeorgeAp left a comment

Choose a reason for hiding this comment

prateeknima77 commented Apr 23, 2024

scampi left a comment

Choose a reason for hiding this comment

scampi Sep 2, 2024

Choose a reason for hiding this comment

prateeknima77 Sep 2, 2024 • edited Loading

Choose a reason for hiding this comment

scampi Sep 5, 2024

Choose a reason for hiding this comment

prateeknima77 Sep 5, 2024

Choose a reason for hiding this comment

GeorgeAp left a comment

Choose a reason for hiding this comment

prateeknima77 Sep 2, 2024 •

edited

Loading