RFC: support lazy all_to_all connection setups #22814

amitmurthy · 2017-07-14T16:04:46Z

This PR does the following:

adds a lazy=true keyword option to addprocs. Default is true. Addresses part of Setup worker-worker connections lazily Distributed.jl#42 via this new keyword arg.
Only applicable to all_to_all connection setups.
All workers are still connected to the master. Only worker-worker connections are setup lazily at the time of the first request between 2 workers.
The lazy option is valid only for all_to_all topologies. For custom topologies, all connections are setup at the time of addprocs.
This more-or-less does away with the need to specify any complex topologies. Connections are setup on demand. For example a stencil operations will only lead to worker-worker connection setups between neighbors.
Will reduce startup time on large clusters.
Will limit resource usage (number of open fds per process) to the minimum required.

Todo:

- Doc update
- Tests

amitmurthy · 2017-07-14T16:06:56Z

cc: @andreasnoack, @JeffBezanson

tkelman · 2017-07-14T16:08:49Z

base/distributed/process_messages.jl

- t = @async connect_to_peer(cluster_manager, rpid, wconfig)
- push!(wait_tasks, t)
+ if lazy
+ # The constructor register the object with a global registery.


registers the object with a global registry

tkelman · 2017-07-14T16:09:29Z

test/distributed_exec.jl

+ i==length(wlist) && continue
+ @async remotecall_fetch(wl -> asyncmap(q->remotecall_fetch(myid, q), wl),
+ p, wlist[i+1:end])
+


unnecessary blank line

tkelman · 2017-07-18T09:54:57Z

base/distributed/process_messages.jl

@@ -317,7 +317,7 @@ function handle_msg(msg::JoinPGRPMsg, header, r_stream, w_stream, version)

 let rpid=rpid, wconfig=wconfig
 if lazy
- # The constructor register the object with a global registery.
+ # The constructor registers the object with a global registery.


registry is still misspelled

tkelman · 2017-07-18T09:55:50Z

test/topology.jl

+
+# Test for 10 random combinations
+wl = workers()
+combinations =[]


combinations = []

tkelman · 2017-07-18T09:56:53Z

test/topology.jl

+ @test num_conns == expected_num_conns
+end
+
+# With lazy=false, all connections ought to be setup initially itself


what are you referring to by "itself" ?

andreasnoack · 2017-07-18T13:32:49Z

I can try this out on a cluster later and see try the effect adding 500-1000 workers. API wise, I think it would be better to have orthogonal options. Is it the plan to make lazy applicable to other topologies later? If not, it might be better to have a :lazyalltoall topology instead.

amitmurthy · 2017-07-18T15:19:59Z

Is it the plan to make lazy applicable to other topologies later? If not, it might be better to have a :lazyalltoall topology instead.

I was planning on doing that, but decided against it for now

If one is going through the the trouble to specify a custom topology then we may as well setup connections upfront
Since specifying custom topologies is currently non-trivial, such use cases are quite well served by lazily setting up connections on demand.

Having said that, a separate keyword arg allows us to enable it across all existing and any new topology types in the future if required.

amitmurthy · 2017-07-24T15:55:38Z

Merging this tomorrow if there are no objections.

tkelman · 2017-07-24T16:11:03Z

base/distributed/cluster.jl

+ if isnull(PGRP.lazy) || nprocs() == 1
+ PGRP.lazy = Nullable{Bool}(params[:lazy])
+ elseif isclusterlazy() != params[:lazy]
+ throw(ErrorException(string("Active workers with lazy=", isclusterlazy(),


probably better as an ArgumentError if this comes from a bad keyword argument

amitmurthy · 2017-07-25T08:31:43Z

We should backport the lazy connection setup behavior onto 0.6 without introducing the lazy keyword, i.e. connections will always be setup lazily under a all_to_all topology. Will not affect any use code.

Opinions?

tkelman · 2017-07-25T08:37:46Z

We don't generally backport behavior changes or new features of this size.

amitmurthy · 2017-07-25T08:43:11Z

The behavior change in this case would be internal. Things would work as before, the only difference being that first time worker-worker requests will include a connection setup time. This is offset by the benefits on larger clusters in terms of initial setup time as well as resource usage if the workload does not require a complete mesh network.

tkelman · 2017-07-25T08:45:35Z

Several types are being changed here in ways that would be visible and breaking for any external code that constructs them directly.

StefanKarpinski · 2017-07-25T20:00:04Z

Is there a particularly strong case as to why we would need this change on 0.6 other than that it's a better default behavior?

amitmurthy · 2017-07-26T06:24:34Z

The strong case is for larger clusters, say 200 workers and above. This will help in situations where a program run does not require all workers to actually communicate with all other workers, but still requires some inter-worker communications (say between neighbors). Defining a custom topology is not straightforward in the cluster manager and a lazy all_to_all setup does away with the need to do so ahead of time.

However, I also realized that this will break interop between 0.6 and 0.6.x and hence is not the right thing to do.

I can make a patch available if and when folks start asking for this in 0.6. People really needing this behavior will need to use a manually patched version of Julia.

tkelman · 2017-07-26T07:17:20Z

Could the backport be implemented by ClusterManagers or some other package? We need to decouple this functionality from the release and compatibility constraints of the Base language and rest of the standard library, the sooner the better.

amitmurthy · 2017-07-26T07:19:50Z

No.

tkelman · 2017-07-26T07:23:20Z

Not an existing package, but this didn't touch anything outside of Distributed, so if that were a package it would be fine.

amitmurthy · 2017-07-26T07:28:18Z

It did change core types and the messaging protocol. Will be difficult to do it externally.

tkelman · 2017-07-26T07:34:21Z

The core types and messaging protocol should be in a package.

sbromberger · 2017-08-30T17:11:49Z

I am strongly in favor of backporting this to 0.6:

see discussion at https://www.researchgate.net/post/What_were_the_reasons_for_selecting_Julia_as_programming_language#59a6d7024048542929566452 for a new user's (mistaken) initial impression of HPC in Julia.
I am unable to run -nightly (or manually patched versions of Julia) in production, and there are significant consequences to non-lazy setup in this environment. Having a "sane" default makes sense.

tkelman reviewed Jul 14, 2017

View reviewed changes

ararslan added the domain:parallelism Parallel or distributed computation label Jul 14, 2017

tkelman reviewed Jul 18, 2017

View reviewed changes

test/topology.jl Outdated

# Test for 10 random combinations

wl = workers()

combinations =[]

Copy link

Contributor

tkelman Jul 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

combinations = []

tkelman reviewed Jul 18, 2017

View reviewed changes

amitmurthy force-pushed the amitm/lazy2 branch from 17c83ca to 9b30e01 Compare July 18, 2017 10:00

amitmurthy changed the title ~~RFC/WIP: support lazy all_to_all connection setups~~ RFC: support lazy all_to_all connection setups Jul 18, 2017

amitmurthy mentioned this pull request Jul 18, 2017

Setup worker-worker connections lazily JuliaLang/Distributed.jl#42

Open

amitmurthy force-pushed the amitm/lazy2 branch from 9b30e01 to f60b2bf Compare July 24, 2017 15:53

tkelman reviewed Jul 24, 2017

View reviewed changes

amitmurthy force-pushed the amitm/lazy2 branch from f60b2bf to c148396 Compare July 25, 2017 05:53

support lazy all_to_all connection setups

f1cc5a1

amitmurthy force-pushed the amitm/lazy2 branch from c148396 to f1cc5a1 Compare July 25, 2017 05:54

amitmurthy merged commit fd951c2 into master Jul 25, 2017

amitmurthy deleted the amitm/lazy2 branch July 25, 2017 08:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC: support lazy all_to_all connection setups #22814

RFC: support lazy all_to_all connection setups #22814

amitmurthy commented Jul 14, 2017 •

edited

Loading

amitmurthy commented Jul 14, 2017

tkelman Jul 14, 2017

tkelman Jul 14, 2017

tkelman Jul 18, 2017

tkelman Jul 18, 2017

tkelman Jul 18, 2017

andreasnoack commented Jul 18, 2017

amitmurthy commented Jul 18, 2017

amitmurthy commented Jul 24, 2017

tkelman Jul 24, 2017

amitmurthy commented Jul 25, 2017

tkelman commented Jul 25, 2017

amitmurthy commented Jul 25, 2017

tkelman commented Jul 25, 2017

StefanKarpinski commented Jul 25, 2017

amitmurthy commented Jul 26, 2017

tkelman commented Jul 26, 2017

amitmurthy commented Jul 26, 2017

tkelman commented Jul 26, 2017

amitmurthy commented Jul 26, 2017

tkelman commented Jul 26, 2017

sbromberger commented Aug 30, 2017 •

edited

Loading

RFC: support lazy all_to_all connection setups #22814

RFC: support lazy all_to_all connection setups #22814

Conversation

amitmurthy commented Jul 14, 2017 • edited Loading

amitmurthy commented Jul 14, 2017

tkelman Jul 14, 2017

Choose a reason for hiding this comment

tkelman Jul 14, 2017

Choose a reason for hiding this comment

tkelman Jul 18, 2017

Choose a reason for hiding this comment

tkelman Jul 18, 2017

Choose a reason for hiding this comment

tkelman Jul 18, 2017

Choose a reason for hiding this comment

andreasnoack commented Jul 18, 2017

amitmurthy commented Jul 18, 2017

amitmurthy commented Jul 24, 2017

tkelman Jul 24, 2017

Choose a reason for hiding this comment

amitmurthy commented Jul 25, 2017

tkelman commented Jul 25, 2017

amitmurthy commented Jul 25, 2017

tkelman commented Jul 25, 2017

StefanKarpinski commented Jul 25, 2017

amitmurthy commented Jul 26, 2017

tkelman commented Jul 26, 2017

amitmurthy commented Jul 26, 2017

tkelman commented Jul 26, 2017

amitmurthy commented Jul 26, 2017

tkelman commented Jul 26, 2017

sbromberger commented Aug 30, 2017 • edited Loading

amitmurthy commented Jul 14, 2017 •

edited

Loading

sbromberger commented Aug 30, 2017 •

edited

Loading