Update arrow version to fix plasma bugs #4127

guoyuhong · 2019-02-22T08:12:06Z

What do these changes do?

This PR updates arrow version to b22848952f09d6f9487feaff80ee358ca41b1562 which contains apache/arrow#3656 and apache/arrow#3682 and this PR will fix 2 TODOs from Plasma Java Client.

Related issue number

N/A

raulchen

LGTM

robertnishihara · 2019-02-22T08:18:54Z

@guoyuhong Did this work for you? @pcmoritz is also updating the Arrow version in #3898 and it's gotten a lot more complex because the Arrow build system has changed.

guoyuhong · 2019-02-22T08:20:00Z

@pcmoritz I rebased the original ref branch arrow-one-mmap-file to the new base of b22848952f09d6f9487feaff80ee358ca41b1562 and resolve the conflicts and push it to arrow-one-mmap-file-on-b228489. Is the processes right?

guoyuhong · 2019-02-22T08:24:22Z

@robertnishihara His PR seems not update the Arrow version. Do you mean that we hold the PR until that PR gets merged?

guoyuhong · 2019-02-22T08:26:23Z

@robertnishihara That PR seems to use Arrow directly from OSS...

robertnishihara · 2019-02-22T08:30:25Z

@guoyuhong I believe it uses Arrow from an S3 bucket that we host. We have to separately build Arrow using https://github.com/ray-project/arrow-build I think, which uploads to the S3 bucket and then we can get it from the bucket. This is done to address issues @pcmoritz ran into when trying to get the Bazel build to work with Arrow.

guoyuhong · 2019-02-22T08:47:35Z

@robertnishihara When will @pcmoritz 's PR get merged? This PR contains 2 fixes along with the Plasma Java Client, so this PR cannot be closed.

robertnishihara · 2019-02-22T08:48:33Z

@guoyuhong I'm hoping to merge @pcmoritz's PR in the morning (in about 8 hours). It's basically ready, but there were a couple small changes that needed to be made.

guoyuhong · 2019-02-22T08:49:37Z

@robertnishihara Thanks for the information. Let's hold this PR until then.

AmplabJenkins · 2019-02-22T09:47:11Z

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

guoyuhong · 2019-02-23T04:04:28Z

@robertnishihara Considering that the Bazel PR is not merged, I think I can send a PR to update https://github.com/ray-project/arrow-build 's Arrow version to 54618466ca04f4427dd8cfc3ce31b14ec54e8999 which is the same as this PR and generate the new wheel files. Then, @pcmoritz 's PR will compatible with this PR using the new wheel files.

pcmoritz · 2019-02-25T09:37:40Z

@guoyuhong Thanks for looking into this, see my comment in ray-project/arrow-build#1

I have now a plan how we can go forward on #4129 and make sure the latest arrow features are available and the clashes with tensorflow are removed. It involves building an arrow wheel that uses ubuntu 14.04 as a base (the same as tensorflow) and I'll prioritize this first (it will allow us to remove CMake) and can help you with this PR after.

guoyuhong · 2019-03-01T03:18:45Z

@pcmoritz #4182 had been merged. How about this PR? Since Java building still goes the cmake way, we need to update is correspondingly. It is better to keep the version of cmake and bazel version the same.

robertnishihara · 2019-03-01T05:36:50Z

@guoyuhong, this PR updates the version of Arrow used by cmake to the commit b22848952f09d6f9487feaff80ee358ca41b1562 which is identical to a commit in the Arrow repository. That means that it does not have the patch ray-project/arrow#3, which was needed to make Ray and TensorFlow compatible.

The tests are passing in this PR probably because when they ran, we were using conda install tensorflow in the Travis tests. However, the current master uses pip install tensorflow in the Travis tests and so I think it may fail (I rebased the PR to check).

If we merge this, we need to use a commit that includes the patch ray-project/arrow#3.

guoyuhong · 2019-03-01T05:42:48Z

@robertnishihara Sorry for the expression. What I mean is that this PR will update to the version that ray-project/arrow#3 rebased on
b22848952f09d6f9487feaff80ee358ca41b1562. And the commit is 54618466ca04f4427dd8cfc3ce31b14ec54e8999.

guoyuhong · 2019-03-01T05:43:13Z

I will do a rebase for this PR to current master.

AmplabJenkins · 2019-03-01T05:47:38Z

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

robertnishihara · 2019-03-01T05:48:36Z

Oh, I apologize, I meant to check the commit from the PR but I guess I checked the one from the commit message. My mistake!

AmplabJenkins · 2019-03-01T06:02:15Z

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

pcmoritz · 2019-03-01T06:07:00Z

Great @guoyuhong thanks for the PR! If it passes on the rebase, let's include it in the point release we are about to finish! Let's however hold off merging big PRs that touch the backend before the release candidate is drafted (besides #4154).

guoyuhong · 2019-03-01T09:39:43Z

There is build error in Arrow CMake and it is fixed by apache/arrow#3737 . I need to rebase the commit to this commit.

robertnishihara · 2019-03-01T20:16:29Z

The std::invalid_argument might be a new error.

AmplabJenkins · 2019-03-01T21:25:08Z

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

guoyuhong · 2019-03-02T03:14:52Z

This PR may help to resolve this problem. apache/arrow#3758

guoyuhong · 2019-03-02T03:24:39Z

I changed the commit to ray-project/arrow@68299c5 of branch arrow-one-mmap-file-on-c0a2e73 which is based on the cmake fix of apache/arrow#3758 .

AmplabJenkins · 2019-03-02T04:45:56Z

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

guoyuhong · 2019-03-02T09:23:47Z

@ericl Since apache/arrow#3423 and the fix apache/arrow#3758 have been merged, the arrow serialization is different. There is a checked-in file https://github.com/ray-project/ray/blob/master/python/ray/rllib/test/data/cartpole_small/output-2019-02-03_20-27-20_worker-0_0.json which is used in Jenkins test but throws exception during deserialization when I try to update arrow version. All the Travis tests passed. Could you help to update that? CC: @pcmoritz
The deserialization code tries to parse ndarray to an int8_t value. Maybe the content of the json file should be updated.

guoyuhong · 2019-03-06T06:11:53Z

@ericl Ping... Could you help to take a look?

ericl · 2019-03-06T06:22:55Z

I see, feel free to disable the test. I can fix it in a follow-up PR.

AmplabJenkins · 2019-03-06T11:48:48Z

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

AmplabJenkins · 2019-03-06T15:42:21Z

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

guoyuhong · 2019-03-07T03:47:05Z

ci/jenkins_tests/run_rllib_tests.sh

@@ -353,8 +355,10 @@ docker run --rm --shm-size=${SHM_SIZE} --memory=${MEMORY_SIZE} $DOCKER_SHA \
 docker run --rm --shm-size=${SHM_SIZE} --memory=${MEMORY_SIZE} $DOCKER_SHA \
    /ray/python/ray/rllib/tests/run_silent.sh examples/cartpole_lstm.py --stop=200 --use-prev-action-reward

-docker run --rm --shm-size=${SHM_SIZE} --memory=${MEMORY_SIZE} $DOCKER_SHA \
-    /ray/python/ray/rllib/tests/run_silent.sh examples/custom_loss.py --iters=2
+# TODO(ericl): reenable the test after fix the arrow serialization error.


@ericl I disabled 3 cases in this script and left a TODO comment for you. Now the Jenkins test has no such failures.

guoyuhong · 2019-03-07T03:49:40Z

Hi all, I have rebased the PR and disable the Jenkins cases that caused by the Arrow serialization change which @ericl promised to fix later. Please help to review.

raulchen

LGTM

robertnishihara · 2019-03-07T06:54:41Z

Looks fine to me assuming tests pass. There is some merge conflict.

AmplabJenkins · 2019-03-07T07:28:05Z

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

AmplabJenkins · 2019-03-07T13:18:04Z

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

…n Arrow 46f75d7

AmplabJenkins · 2019-03-08T07:34:40Z

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

guoyuhong requested review from pcmoritz, raulchen and robertnishihara and removed request for pcmoritz February 22, 2019 08:12

guoyuhong force-pushed the updateArrow branch from c3780e4 to 7bbcfa8 Compare February 22, 2019 08:15

raulchen approved these changes Feb 22, 2019

View reviewed changes

guoyuhong mentioned this pull request Feb 23, 2019

Update Arrow to arrow-one-mmap-file-on-b228489 ray-project/arrow-build#1

Closed

robertnishihara force-pushed the updateArrow branch from 7bbcfa8 to 98ec5e1 Compare March 1, 2019 05:32

guoyuhong force-pushed the updateArrow branch from 98ec5e1 to 0d071d4 Compare March 1, 2019 05:43

guoyuhong force-pushed the updateArrow branch from e8be6ac to 8cd9680 Compare March 6, 2019 09:06

guoyuhong force-pushed the updateArrow branch from 8cd9680 to 70210ff Compare March 6, 2019 11:53

guoyuhong force-pushed the updateArrow branch from 70210ff to c7edd6c Compare March 7, 2019 03:45

guoyuhong commented Mar 7, 2019

View reviewed changes

raulchen approved these changes Mar 7, 2019

View reviewed changes

guoyuhong force-pushed the updateArrow branch from c7edd6c to 5d86cd8 Compare March 7, 2019 07:54

guoyuhong added 5 commits March 8, 2019 12:02

Update arrow

bcd6602

Change to 2c511979b13b230e73a179dab1d55b03cd81ec02 which is rebased o…

6d3da09

…n Arrow 46f75d7

Update to fix comment

0f1160e

disable tests which use python/ray/rllib/tests/data/cartpole_small

6d4773b

Fix get order of meta and data in MockObjectStore.java

c7a3385

guoyuhong force-pushed the updateArrow branch from 5d86cd8 to c7a3385 Compare March 8, 2019 04:34

guoyuhong merged commit d5fb7b7 into ray-project:master Mar 8, 2019

guoyuhong deleted the updateArrow branch March 8, 2019 10:04

pcmoritz mentioned this pull request Mar 14, 2019

Current master ray wheel doesn't work #4356

Closed

Update arrow version to fix plasma bugs #4127

Update arrow version to fix plasma bugs #4127

Conversation

guoyuhong commented Feb 22, 2019

What do these changes do?

Related issue number

raulchen left a comment

Choose a reason for hiding this comment

robertnishihara commented Feb 22, 2019

guoyuhong commented Feb 22, 2019

guoyuhong commented Feb 22, 2019

guoyuhong commented Feb 22, 2019

robertnishihara commented Feb 22, 2019

guoyuhong commented Feb 22, 2019

robertnishihara commented Feb 22, 2019

guoyuhong commented Feb 22, 2019

AmplabJenkins commented Feb 22, 2019

guoyuhong commented Feb 23, 2019

pcmoritz commented Feb 25, 2019

guoyuhong commented Mar 1, 2019

robertnishihara commented Mar 1, 2019

guoyuhong commented Mar 1, 2019

guoyuhong commented Mar 1, 2019

AmplabJenkins commented Mar 1, 2019

robertnishihara commented Mar 1, 2019

AmplabJenkins commented Mar 1, 2019

pcmoritz commented Mar 1, 2019 • edited Loading

guoyuhong commented Mar 1, 2019

robertnishihara commented Mar 1, 2019

AmplabJenkins commented Mar 1, 2019

guoyuhong commented Mar 2, 2019

guoyuhong commented Mar 2, 2019

AmplabJenkins commented Mar 2, 2019

guoyuhong commented Mar 2, 2019 • edited Loading

guoyuhong commented Mar 6, 2019

ericl commented Mar 6, 2019

AmplabJenkins commented Mar 6, 2019

AmplabJenkins commented Mar 6, 2019

guoyuhong Mar 7, 2019

Choose a reason for hiding this comment

guoyuhong commented Mar 7, 2019

raulchen left a comment

Choose a reason for hiding this comment

robertnishihara commented Mar 7, 2019

AmplabJenkins commented Mar 7, 2019

AmplabJenkins commented Mar 7, 2019

AmplabJenkins commented Mar 8, 2019

pcmoritz commented Mar 1, 2019 •

edited

Loading

guoyuhong commented Mar 2, 2019 •

edited

Loading