Derecho-Project / derecho

The main code repository for the Derecho project.
BSD 3-Clause "New" or "Revised" License
186 stars 47 forks source link

JSON-based layout #206

Closed etremel closed 3 years ago

etremel commented 3 years ago

This is @Panlichen's implementation of JSON descriptions for Derecho layout, slightly improved and debugged by me. Subgroups configured via JSON can specify "reserved node IDs" that they should always be assigned if those nodes exist; this will override the default allocation algorithm that assigns nodes to subgroups in a round-robin fashion.

Panlichen commented 3 years ago

Weijia mentioned your plan in our meeting on Tuesday. Thanks a lot for your effort! I'll learn your code tomorrow(UTC+8).

songweijia commented 3 years ago

Thank you, guys! I reviewed the code today. It looks great. The only thing which might be a little confusing to users is the JSON layout configuration.

Since we introduce the "json_layout" into the configuration file, should we allow the template function make_subgroup_allocator to read from the JSON configuration entry (or JSON configuration file) directly instead of accepting a JSON object? For example, we can have a wrapper function called make_subgroup_allocator_by_json_layout(), which will load the JSON layout defined by the derecho configuration file. The users do not need to load it by themselves. Further, I suggest getting rid of the json_layout option and only keep the json_layout_file option.

Another option is to remove all JSON layout options from the configuration file. The users should be responsible for loading the configuration from wherever they define it.

Whichever option you take, you should also add a document for the format of the JSON layout in a sample file like what we did in src/conf/sample-derecho.cfg.

What do you think?

etremel commented 3 years ago

That's a good idea, Weijia, and in fact I already implemented something like it. There's a no-arguments version of make_subgroup_allocator that automatically reads the JSON configuration from the config file, assuming that either the json_layout or json_layout_path options have been configured. It's defined here:

https://github.com/Derecho-Project/derecho/blob/bb8f7cc881c1bd6be7be9367044367e99540bfeb/include/derecho/core/subgroup_functions.hpp#L485-L488

and wraps this constructor for DefaultSubgroupAllocator:

https://github.com/Derecho-Project/derecho/blob/bb8f7cc881c1bd6be7be9367044367e99540bfeb/src/core/subgroup_functions.cpp#L647-L672

Does this provide the functionality you were looking for?

songweijia commented 3 years ago

Yes, that's exactly what I'm looking for. I found the example code which passed the JSON layout explicitly but didn't realize that you have it already. The only thing we need is a sample file to document the JSON format. After that, let's merge it and I will change the Cascade code accordingly.

etremel commented 3 years ago

Sorry for the delay in following up, but I added some documentation to README.md and merged this branch into master. (I was distracted with the other issue and forgot about this one). You can go ahead and start using this in Cascade.