Incorporate C++ Buffer management and Seal global threadpool fix from arrow #1950

pcmoritz · 2018-04-24T22:27:52Z

This includes apache/arrow#1939 and gets rid of the revert of apache/arrow#1807

It includes the following relevant arrow improvements:

Use the automatic C++ PlasmaBuffer management for the Python wrappers (i.e. Release is called automatically)
Removes the Plasma seal global threadpool which caused problems if multiple PlasmaClients were used in the same process in different threads

AmplabJenkins · 2018-04-24T23:25:29Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/5055/
Test PASSed.

AmplabJenkins · 2018-04-24T23:34:53Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/5056/
Test PASSed.

AmplabJenkins · 2018-04-25T00:59:39Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/5059/
Test PASSed.

AmplabJenkins · 2018-04-25T07:14:31Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/5064/
Test PASSed.

AmplabJenkins · 2018-04-25T07:21:31Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/5067/
Test FAILed.

AmplabJenkins · 2018-04-25T08:12:50Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/5068/
Test PASSed.

AmplabJenkins · 2018-04-25T19:55:25Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/5072/
Test PASSed.

AmplabJenkins · 2018-04-25T22:36:58Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/5074/
Test FAILed.

AmplabJenkins · 2018-04-26T01:49:25Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/5075/
Test PASSed.

robertnishihara · 2018-04-26T03:16:07Z

src/common/cmake/Common.cmake

@@ -10,7 +10,12 @@ if(UNIX AND NOT APPLE)
  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -rdynamic")
 endif()

-set(FLATBUFFERS_VERSION "1.7.1")
+# The following is needed because in CentOS, the lib directory is named lib64
+if(EXISTS "/etc/redhat-release" AND CMAKE_SIZEOF_VOID_P EQUAL 8)


Can we use this to get rid of

ray/thirdparty/scripts/build_arrow.sh

Lines 85 to 90 in dad465a

if [[ -d $ARROW_HOME/lib64 ]]; then

# On CentOS, Arrow gets installed under lib64 instead of lib, so copy it for

# now. TODO(rkn): A preferable solution would be to add both directories to

# the PKG_CONFIG_PATH, but that didn't seem to work.

cp -r $ARROW_HOME/lib64 $ARROW_HOME/lib

fi

and related lines?

Relatedly, is this the canonical way of checking for CentOS? https://askubuntu.com/questions/459402/how-to-know-if-the-running-platform-is-ubuntu-or-centos-with-help-of-a-bash-scri suggests /etc/os-release.

I took it from https://htcondor-wiki.cs.wisc.edu/index.cgi/fileview?f=build/cmake/FindLinuxPlatform.cmake&v=4592599fecc08e5588c4244e2b0ceb7d32363a56, it's officially documented by Red Hat, so should be fine.

Getting rid of the copy of the arrow lib64 will involve changing FindArrow, which is part of arrow.

robertnishihara · 2018-04-26T03:24:04Z

thirdparty/scripts/build_flatbuffers.sh

@@ -7,7 +7,7 @@ set -e

 TP_DIR=$(cd "$(dirname "${BASH_SOURCE:-$0}")"; pwd)/../

-FLATBUFFERS_VERSION=1.7.1


Wait, are we building flatbuffers twice? (excluding the time that Arrow builds it)

Why do we have this script in addition to the ExternalProject_Add(flatbuffers_ep in Common.cmake?

Yeah, I think we can figure this out when we get rid of the old ray code (and get rid of Common.cmake then).

robertnishihara · 2018-04-26T03:26:11Z

Can you make the PR title and description more informative? E.g., what changes are we getting from Arrow?

* master: updates (ray-project#1958) Pin Cython in autoscaler development example. (ray-project#1951) Incorporate C++ Buffer management and Seal global threadpool fix from arrow (ray-project#1950) [XRay] Add consistency check for protocol between node_manager and local_scheduler_client (ray-project#1944) Remove smart_open install. (ray-project#1943) [DataFrame] Fully implement append, concat and join (ray-project#1932) [DataFrame] Fix for __getitem__ string indexing (ray-project#1939) [DataFrame] Implementing write methods (ray-project#1918) [rllib] arr[end] was excluded when end is not None (ray-project#1931) [DataFrame] Implementing API correct groupby with aggregation methods (ray-project#1914) Handle interrupts correctly for ASIO synchronous reads and writes. (ray-project#1929) [DataFrame] Adding read methods and tests (ray-project#1712) Allow task_table_update to fail when tasks are finished. (ray-project#1927) [rllib] Contribute DDPG to RLlib (ray-project#1877) [xray] Workers blocked in a `ray.get` release their resources (ray-project#1920) Raylet task dispatch and throttling worker startup (ray-project#1912) [DataFrame] Eval fix (ray-project#1903)

* 'master' of https://github.com/ray-project/ray: [rllib] Fix broken link in docs (ray-project#1967) [DataFrame] Sample implement (ray-project#1954) [DataFrame] Implement Inter-DataFrame operations (ray-project#1937) remove UniqueIDHasher (ray-project#1957) [rllib] Add DDPG documentation, rename DDPG2 <=> DDPG (ray-project#1946) updates (ray-project#1958) Pin Cython in autoscaler development example. (ray-project#1951) Incorporate C++ Buffer management and Seal global threadpool fix from arrow (ray-project#1950) [XRay] Add consistency check for protocol between node_manager and local_scheduler_client (ray-project#1944) Remove smart_open install. (ray-project#1943) [DataFrame] Fully implement append, concat and join (ray-project#1932) [DataFrame] Fix for __getitem__ string indexing (ray-project#1939) [DataFrame] Implementing write methods (ray-project#1918) [rllib] arr[end] was excluded when end is not None (ray-project#1931) [DataFrame] Implementing API correct groupby with aggregation methods (ray-project#1914)

pcmoritz added 11 commits April 23, 2018 23:28

upgrade to latest version of arrow

cb973cb

fix XRay object manager

abad8af

fix commit

ccbe26c

fix object manager tests

aea241b

fix macOS build error

3567b6b

upgrade flatbuffers

5cda33d

fix flatbuffer version

111bbb3

fix flatbuffers install

8a79e2b

fix

f1f22ad

fix

2f46b3a

update

384824a

fix

042f00d

pcmoritz added 3 commits April 24, 2018 23:09

fixfix

87e548a

fixfixfix

ada1c78

fix arrow pointer

d4f11fa

update

baab66e

update

1289891

fix manylinux1 build

3be96e9

robertnishihara reviewed Apr 26, 2018

View reviewed changes

pcmoritz changed the title ~~Upgrade arrow to latest version~~ Incorporate C++ Buffer management and Seal global threadpool fix from arrow Apr 26, 2018

robertnishihara approved these changes Apr 26, 2018

View reviewed changes

robertnishihara merged commit af88fde into ray-project:master Apr 26, 2018

robertnishihara deleted the upgrade-arrow branch April 26, 2018 05:53

pcmoritz mentioned this pull request Apr 26, 2018

[XRay] Fix PlasmaClient multithreading issue #1900

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Incorporate C++ Buffer management and Seal global threadpool fix from arrow #1950

Incorporate C++ Buffer management and Seal global threadpool fix from arrow #1950

pcmoritz commented Apr 24, 2018 •

edited

Loading

AmplabJenkins commented Apr 24, 2018

AmplabJenkins commented Apr 24, 2018

AmplabJenkins commented Apr 25, 2018

AmplabJenkins commented Apr 25, 2018

AmplabJenkins commented Apr 25, 2018

AmplabJenkins commented Apr 25, 2018

AmplabJenkins commented Apr 25, 2018

AmplabJenkins commented Apr 25, 2018

AmplabJenkins commented Apr 26, 2018

robertnishihara Apr 26, 2018

robertnishihara Apr 26, 2018

pcmoritz Apr 26, 2018

pcmoritz Apr 26, 2018 •

edited

Loading

robertnishihara Apr 26, 2018

pcmoritz Apr 26, 2018

robertnishihara commented Apr 26, 2018

	if [[ -d $ARROW_HOME/lib64 ]]; then
	# On CentOS, Arrow gets installed under lib64 instead of lib, so copy it for
	# now. TODO(rkn): A preferable solution would be to add both directories to
	# the PKG_CONFIG_PATH, but that didn't seem to work.
	cp -r $ARROW_HOME/lib64 $ARROW_HOME/lib
	fi

		@@ -7,7 +7,7 @@ set -e

		TP_DIR=$(cd "$(dirname "${BASH_SOURCE:-$0}")"; pwd)/../

		FLATBUFFERS_VERSION=1.7.1

Incorporate C++ Buffer management and Seal global threadpool fix from arrow #1950

Incorporate C++ Buffer management and Seal global threadpool fix from arrow #1950

Conversation

pcmoritz commented Apr 24, 2018 • edited Loading

AmplabJenkins commented Apr 24, 2018

AmplabJenkins commented Apr 24, 2018

AmplabJenkins commented Apr 25, 2018

AmplabJenkins commented Apr 25, 2018

AmplabJenkins commented Apr 25, 2018

AmplabJenkins commented Apr 25, 2018

AmplabJenkins commented Apr 25, 2018

AmplabJenkins commented Apr 25, 2018

AmplabJenkins commented Apr 26, 2018

robertnishihara Apr 26, 2018

Choose a reason for hiding this comment

robertnishihara Apr 26, 2018

Choose a reason for hiding this comment

pcmoritz Apr 26, 2018

Choose a reason for hiding this comment

pcmoritz Apr 26, 2018 • edited Loading

Choose a reason for hiding this comment

robertnishihara Apr 26, 2018

Choose a reason for hiding this comment

pcmoritz Apr 26, 2018

Choose a reason for hiding this comment

robertnishihara commented Apr 26, 2018

pcmoritz commented Apr 24, 2018 •

edited

Loading

pcmoritz Apr 26, 2018 •

edited

Loading