Thanks Jangho for the explanation. I think I have a better understanding of the problem now.
The SchedulingPolicy we have directly references the 'mechanisms' for executors and tasks. Nonetheless, I'd define SchedulingPolicy as 'given some information about a group of IRVertices (Stage) and executors, return executors that we want to use'. As you suggested, introducing a Stage interface would be nice, as we can do something like filter(StageInterface, list(ExecutorInfo)) which can be implemented and configured in the optimizer. The runtime can supply appropriate arguments when calling the method, without exposing its 'mechanisms'. However, I'd be careful about introducing a concrete Stage 'class' in the optimizer, as that may require modifications of the stage partitioning logic, and optimization passes that only knows about IRVertices. Alternatively, and as the first implementation goal, something like this may be nice. filter(ExecutionPropertyMap, list(ExecutorInfo)) which gives us the minimal information we need about the group of vertices and the executors. We can then tag an implementation of this method to each IRVertice, and perform checks when assigning stage numbers, similar to how we currently check if vertices of the same stage have the same degree of parallelism. After that I guess we can think about using a Stage interface, or providing additional information for expressing more sophisticated policies such as straggler handling. On a minor note, SchedulingPolicy appears to be in the master package, and not in the runtime-common package. https://github.com/apache/incubator-nemo/blob/master/runtime/master/src/main/java/edu/snu/nemo/runtime/master/scheduler/SchedulingPolicy.java John On Fri, Jun 1, 2018, 5:59 PM JangHo Seo <[email protected]> wrote: > Thanks for the opinion. > > Currently some of policy-related code(e.g. SchedulingPolicy) depend on > mechanism-related code(e.g. PhysicalStage). > That said, SchedulingPolicy depends on 'declaration' of PhysicalStage, > not the actual 'implementation' of it. > And I agree mechanism-related code should stay within nemo-runtime-* > module. > > Maybe we can add PhysicalStage as an interface in nemo-common and rename > the current PhysicalStage to PhysicalStageImpl, implementing the > PhyscalStage interface. > > Regards, > Jangho > > > On 06/01/2018 05:49 PM, John Yang wrote: > > Sounds good to me. > > > > > > I agree that policy-related code should go into 'nemo-common' for the > > compiler-optimizer to optimize, and the runtime to execute. > > I guess 'KeyExtractor' and the ir package belong to this category, and > > that's why they're in 'nemo-common' > > > > On the other hand, mechanism-related code should stay in > > 'nemo-runtime-common' and should be exposed only to the > > compiler-backend/nemo. > > Things like 'PhysicalStage' seems to belong to this category, as it'd be > > unlikely that compiler-optimizer would want to do something with it. > > > > > > Thanks, > > John > > > > On Fri, Jun 1, 2018 at 3:59 PM, JangHo Seo <[email protected]> wrote: > > > >> Hi Nemo devs, > >> > >> Currently Nemo codebase is splitted into multiple Maven modules. > >> While it's good for ease of maintenance and modularity, a module cannot > >> reference classes and/or interfaces in other modules unless it declares > >> them as dependencies in pom.xml. > >> Sometimes it makes difficult to expose runtime-level behaviors as > >> IR-level abstractions, when nemo-compiler-* modules cannot access the > >> related definitions in nemo-runtime-* modules. > >> > >> A good example is [NEMO-73] 'SchedulingPolicy as Vertex-levelExecution > >> Property'. The issue is to expose SchedulingPolicy to compiler passes so > >> that users can configure scheduling behavior using Nemo IR. > >> Unfortunately 'SchedulingPolicy' is defined in 'nemo-runtime-common', so > >> compilers cannot access it and I think the proper fix is to move it to > >> the 'nemo-common' module. > >> Also, to move 'SchedulingPolicy' to 'nemo-common' module, lots of > >> definitions in 'nemo-runtime-common' should also be moved to > 'nemo-common'. > >> (Because 'SchedulingPolicy' is a function of 'ExecutorRepresenter's and > >> 'ExecutableTask', and the definition of 'ExecutableTask' depends on > >> 'PhysicalStageEdge', 'PhysicalStage', and 'KeyRange', and so on...) > >> > >> The module 'nemo-runtime-common' has definitions that define 'policies', > >> as well as definitions that implement 'mechanisms'. > >> 'KeyRange' is an example for the former (defines how to partition the > >> data in a block), and 'MessageListener' is an example for the latter > >> (implements RPC). > >> In my opinion, policy-related definitions should be moved to > >> nemo-common, so that we can add more ExecutionProperties about > >> scheduilng, dynamic optimization, state management, and so on. > >> > >> What do you think about this issue? > >> > >> Regards, > >> Jangho > >> > >> > > > >
