Revert "Revert "[Core] Support Arrow zerocopy serialization in object… #36153

Deegue · 2023-06-07T08:46:25Z

This reverts commit 822904b.

Why are these changes needed?

Last PR was reverted since test_advanced_9 failed. However, I tested test_advanced_9 on local device and it passed.

This assert failure looks unrelated to this PR.

Lines 426 to 430 in 0b190ee

    
           import numpy  # noqa: F401 
        
           from threadpoolctl import threadpool_info 
        
           for pool_info in threadpool_info(): 
        
               assert pool_info["num_threads"] == 2

Related issue number

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.io/en/master/.
- I've added any new APIs to the API Reference. For example, if I added a
  method in Tune, I've added it in doc/source/tune/api/ under the
  corresponding .rst file.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(

… store (ray-project#35110)" (ray-project#36000)" This reverts commit 822904b.

Deegue · 2023-06-07T10:15:38Z

It's weird test_advanced_9 of (Medium A-J) failed in CI but worked well on my local device..

Deegue · 2023-06-12T02:10:25Z

Gentle ping @ericl @rkooo567 for help, thanks~

ericl · 2023-06-12T17:35:42Z

Not really sure, but perhaps try disabling parts of your PR to see what is causing the issue? It may be worth adding more debug logs for where OMP_NUM_THREADS is being set as well.

rickyyx · 2023-07-06T17:11:25Z

python/ray/tests/test_advanced_9.py

@@ -420,7 +420,7 @@ def test_omp_threads_set_third_party(ray_start_cluster, monkeypatch):
        cluster.add_node(num_cpus=4)
        ray.init(address=cluster.address)

-        @ray.remote(num_cpus=2)
+        @ray.remote(num_cpus=1)


This is the reason that it broke the test I guess?

We use num_cpus for OMP_NUM_THREADS for a task - why did it get changed?

Another possible reason that it might break is if any new import introduced in the PR eagerly imports numpy.

Thanks for your review, this change is for test only, and it won't affect the result since I tested before.

This PR imports pyarrow, yet I cannot get the relationship with the break.. Could you find where the 4 comes from in the break assertation?

likely the 4 comes from the num_cpus passed into init.

Looks like it's fixed on the current HEAD?

Yes, I found it a little bit tricky since it's fixed after I moved import pyarrow into functions..

Some other cases failed, and I have no idea whether theses are related..

you could ignore the metric heads test - these should be fixed in master btw.

The arrow test failure looks relevant?

Got it , just found WorkerCrashedError in Arrow 6, it seems no details.

… raydp_ser_revert

Deegue · 2023-07-12T06:50:10Z

Could we also tag other reviews who have context on the original PR for review? @Deegue

And let me know if there's anything I can help or I don't think I am the best person to approve it.

Ok, here is the original PR #35110 and gentle ping @ericl for review since the previous failed case was passed.

Thanks again @rickyyx , could you help to find the fail reason of Arrow 6? I'm not sure if it's because of import pyarrow since there is no stacktrace.

Deegue · 2023-07-12T06:56:05Z

can you let me know what were the major changes from the PR that was merged?

Thanks for comment, the major change is using arrow serialization for object store read and write, which is zero-copy and brings better performance.

rickyyx · 2023-07-12T17:18:03Z

I got this stacktrace from one of the workers in the failed test for arrow 6

*** SIGSEGV received at time=1689148296 on cpu 1 ***
PC: @     0x7fdda5605b0c  (unknown)  arrow::(anonymous namespace)::NullArrayFactory::CreateChild()
    @     0x7fdde91d81d5        208  absl::lts_20220623::WriteFailureInfo()
    @     0x7fdde91d7f18         64  absl::lts_20220623::AbslFailureSignalHandler()
    @     0x7fddea56c420  (unknown)  (unknown)
    @     0x7fddea2a45f3        160  (unknown)
    @     0x7fdda56064a7        208  arrow::VisitTypeInline<>()
    @ ... and at least 1 more frames
[2023-07-12 07:51:36,709 E 19106 19106] logging.cc:361: *** SIGSEGV received at time=1689148296 on cpu 1 ***
[2023-07-12 07:51:36,709 E 19106 19106] logging.cc:361: PC: @     0x7fdda5605b0c  (unknown)  arrow::(anonymous namespace)::NullArrayFactory::CreateChild()
[2023-07-12 07:51:36,709 E 19106 19106] logging.cc:361:     @     0x7fdde91d81d5        208  absl::lts_20220623::WriteFailureInfo()
[2023-07-12 07:51:36,710 E 19106 19106] logging.cc:361:     @     0x7fdde91d7f31         64  absl::lts_20220623::AbslFailureSignalHandler()
[2023-07-12 07:51:36,710 E 19106 19106] logging.cc:361:     @     0x7fddea56c420  (unknown)  (unknown)
[2023-07-12 07:51:36,711 E 19106 19106] logging.cc:361:     @     0x7fddea2a45f3        160  (unknown)
[2023-07-12 07:51:36,711 E 19106 19106] logging.cc:361:     @     0x7fdda56064a7        208  arrow::VisitTypeInline<>()
[2023-07-12 07:51:36,711 E 19106 19106] logging.cc:361:     @ ... and at least 1 more frames
Fatal Python error: Segmentation fault

Stack (most recent call first):
  File "/opt/miniconda/lib/python3.8/site-packages/pyarrow/compute.py", line 625 in take
  File "/ray/python/ray/data/_internal/arrow_ops/transform_pyarrow.py", line 41 in take_table
  File "/ray/python/ray/data/_internal/arrow_block.py", line 320 in take
  File "/ray/python/ray/data/_internal/arrow_block.py", line 220 in random_shuffle
  File "/ray/python/ray/data/_internal/shuffle_and_partition.py", line 88 in reduce
  File "/ray/python/ray/_private/worker.py", line 777 in main_loop
  File "/ray/python/ray/_private/workers/default_worker.py", line 262 in <module>

Signed-off-by: Zhi Lin <[email protected]>

kira-lin · 2023-07-26T06:01:30Z

@rickyyx I'm helping to fix the CI. The arrow related tests are passed. Other failures seems unrelated?

Signed-off-by: Zhi Lin <[email protected]>

kira-lin · 2023-07-31T05:07:25Z

Hi @ericl @rkooo567 , please review again, thanks

ericl · 2023-07-31T22:27:37Z

Core tests look good now. Can you rebase with master to rerun the Java tests? It looks like

java.lang.NoClassDefFoundError: com/fasterxml/jackson/core/util/JacksonFeature

is a potential dependency issue.

kira-lin · 2023-08-11T03:55:30Z

hi @jovany-wang ,
Do you have any idea about this NoClassDefFoundError? We only add a few Arrow dependencies, I don't see how this error is related to this PR. Thanks for your help

stale · 2023-09-16T22:16:29Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.

stale · 2023-12-15T07:00:03Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.

Revert "Revert "[Core] Support Arrow zerocopy serialization in object…

95986a4

… store (ray-project#35110)" (ray-project#36000)" This reverts commit 822904b.

Deegue requested review from ericl, scv119, c21, amogkam, scottjlee, bveeramani, raulchen, jovany-wang, kfstorm, fishbone and WangTaoTheTonic as code owners June 7, 2023 08:46

Deegue added 5 commits June 13, 2023 01:40

test

015b2e5

test

d6d3794

test

709ad05

test

95f92d0

test

bd9c9af

rickyyx self-assigned this Jun 27, 2023

Deegue and others added 2 commits June 28, 2023 02:13

revert test log

f00591a

Merge branch 'ray-project:master' into raydp_ser_revert

9097771

rickyyx reviewed Jul 6, 2023

View reviewed changes

rickyyx added the @external-author-action-required Alternate tag for PRs where the author doesn't have labeling permission. label Jul 6, 2023

Deegue and others added 5 commits July 7, 2023 02:32

move import inside

ad28538

Merge branch 'raydp_ser_revert' of https://github.com/Deegue/ray into…

e761683

… raydp_ser_revert

nit

c3e5db3

lint and fix test

03e64d6

Merge branch 'ray-project:master' into raydp_ser_revert

dd10dab

lint

e390b6a

Deegue and others added 8 commits July 20, 2023 03:35

move inside

c02b243

nit

e6ae146

revert import in transform_pyarrow

6aa4ad8

Signed-off-by: Zhi Lin <[email protected]>

comment added part and see if ci pass

c47572b

Signed-off-by: Zhi Lin <[email protected]>

see if import breaks ci

3da0729

Signed-off-by: Zhi Lin <[email protected]>

add back deser part

5872b76

Signed-off-by: Zhi Lin <[email protected]>

add back serialize import

5def2f9

Signed-off-by: Zhi Lin <[email protected]>

return table if indices is empty

371224d

Signed-off-by: Zhi Lin <[email protected]>

kira-lin added 2 commits July 26, 2023 13:29

add empty check for take_table

94ed413

Signed-off-by: Zhi Lin <[email protected]>

Merge remote-tracking branch 'upstream/master' into raydp_ser_revert

7bd21d3

Deegue added 2 commits August 1, 2023 09:48

Merge branch 'ray-project:master' into raydp_ser_revert

b53fc29

Merge branch 'ray-project:master' into raydp_ser_revert

9b8768d

Deegue requested a review from SongGuyang as a code owner August 8, 2023 03:23

stale bot added the stale The issue is stale. It will be closed within 7 days unless there are further conversation label Sep 16, 2023

rkooo567 removed the stale The issue is stale. It will be closed within 7 days unless there are further conversation label Oct 3, 2023

stale bot added the stale The issue is stale. It will be closed within 7 days unless there are further conversation label Dec 15, 2023

Deegue requested review from stephanie-wang and Zandew as code owners December 25, 2023 04:13

stale bot removed stale The issue is stale. It will be closed within 7 days unless there are further conversation labels Dec 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert "Revert "[Core] Support Arrow zerocopy serialization in object… #36153

Revert "Revert "[Core] Support Arrow zerocopy serialization in object… #36153

Deegue commented Jun 7, 2023

Deegue commented Jun 7, 2023

Deegue commented Jun 12, 2023

ericl commented Jun 12, 2023

rickyyx Jul 6, 2023

rickyyx Jul 6, 2023

Deegue Jul 7, 2023

rickyyx Jul 7, 2023

rickyyx Jul 7, 2023

Deegue Jul 8, 2023

Deegue Jul 8, 2023

rickyyx Jul 8, 2023

Deegue Jul 10, 2023 •

edited

Loading

Deegue commented Jul 12, 2023

Deegue commented Jul 12, 2023

rickyyx commented Jul 12, 2023 •

edited

Loading

kira-lin commented Jul 26, 2023

kira-lin commented Jul 31, 2023

ericl commented Jul 31, 2023

kira-lin commented Aug 11, 2023

stale bot commented Sep 16, 2023

stale bot commented Dec 15, 2023

	import numpy # noqa: F401
	from threadpoolctl import threadpool_info

	for pool_info in threadpool_info():
	assert pool_info["num_threads"] == 2

Revert "Revert "[Core] Support Arrow zerocopy serialization in object… #36153

Are you sure you want to change the base?

Revert "Revert "[Core] Support Arrow zerocopy serialization in object… #36153

Conversation

Deegue commented Jun 7, 2023

Why are these changes needed?

Related issue number

Checks

Deegue commented Jun 7, 2023

Deegue commented Jun 12, 2023

ericl commented Jun 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Deegue Jul 10, 2023 • edited Loading

Choose a reason for hiding this comment

Deegue commented Jul 12, 2023

Deegue commented Jul 12, 2023

rickyyx commented Jul 12, 2023 • edited Loading

kira-lin commented Jul 26, 2023

kira-lin commented Jul 31, 2023

ericl commented Jul 31, 2023

kira-lin commented Aug 11, 2023

stale bot commented Sep 16, 2023

stale bot commented Dec 15, 2023

Deegue Jul 10, 2023 •

edited

Loading

rickyyx commented Jul 12, 2023 •

edited

Loading