LGTM. And I like the 123MB more than separating value and unit into two settings.
Quick questions: This new config will replace the existing topology.component.rammap? "the way ECO handles topology configuration will not work for all configuration types". Can you give a more specific example? Thanks. On Mon, Jan 22, 2018 at 9:33 AM, Karthik Ramasamy <[email protected]> wrote: > Josh - > > One more feedback - since the resources assigned can be CPU, RAM, DISK - > instead of calling it > > topology.component.rammap > > can we call it > > topology.component.resourcemap > > and allow for CPU and DISK. Furthermore, we append the size type into the > metric as follows > > config: > topology.workers: 2 > topology.component.resourcemap: > > - id: "component-1" > ram: 1234MB > cpu: 0.5 > disk: 123MB > > - id: "component-2" > ram: 2345MB > cpu: 0.75 > disk: 4GB > > This will make it easier to read and also flexible, thoughts? > > cheers > /karthik > > > > cheers > /karthik > > On Sun, Jan 21, 2018 at 6:18 PM, Josh Fischer <[email protected]> wrote: > > > To All, > > > > I think I made a mistake in my previous email > > > > config: > > topology.workers: 2 > > topology.component.rammap: > > - "some-id": 1234 > > - "other-id": 6789 > > > > > > I think the yaml above is incorrect as well as other examples. I think > we > > would have to do something like below > > > > > > config: > > topology.workers: 2 > > topology.component.rammap: > > - "some-id:1234" > > - "other-id:6789" > > > > Which would then product a list of strings that would match the way the > > topology_component_rammap is set via other apis. The problem with this > > approach is it would be easy for someone to make a mistake within the > > formatting of the strings and would then cause us to have to validate the > > format to fit the specs. I think the approach below would be better. I > > would then just take the input, do some validation and conversion via the > > ByteAmount class and generate a properly formatted string to fit the > specs > > of the topology_component_rammap values. > > > > config: > > topology.workers: 2 > > topology.component.rammap: > > > > - id: "component-1" > > size: 1234 > > type: MB // Megabytes > > > > - id: "component-2" > > size: 6789 > > type: GB // GigaBytes > > > > - id: "component-3" > > size: 123456789 > > type: B // Bytes > > > > > > > > Hope I was clear with trying to explain things. Of course I will also be > > creating the docs as well to explain usage. > > > > -Josh > > > > On Sun, Jan 21, 2018 at 8:21 AM, Josh Fischer <[email protected]> > wrote: > > > > > All, > > > > > > While working with Karthik, we have discovered that the way ECO handles > > > topology configuration will not work for all configuration types. To > be > > > specific, setting individual component's ram will not work. We will > also > > > have to keep in mind container size that contains the components. My > > > proposal is this: > > > > > > Create a standardized way to allow for the configuring of component > ram > > > size in the "config" section of the eco yaml file. This would be a > list > > > of key value pairs that mapped the "id" of a component to an allocated > > ram > > > size in MB. An example is below: > > > > > > config: > > > topology.workers: 2 > > > topology.component.rammap: > > > - "some-id": 1234 > > > - "other-id": 6789 > > > > > > However the above implementation may be unclear when it comes to > > > understanding what unit of measurement is implicitly specified and/or > > > expected. Or we could do something like below. > > > > > > config: > > > topology.workers: 2 > > > topology.component.rammap: > > > - spec: > > > id: "component-1" > > > size: 1234 > > > type: MB // Megabytes > > > - spec: > > > id: "component-2" > > > size: 6789 > > > type: GB // GigaBytes > > > - spec: > > > id: "component-3" > > > size: 123456789 > > > type: B // Bytes > > > > > > > > > If a mapping is not specified for a component, we can just assume > Heron's > > > defaults. We could then dynamically calculate the container size based > > off > > > of the number components and their corresponding allocated resources > for > > > simplicity of use for the user, but still allow them to specify a > custom > > > set of resources to a container like below > > > > > > topology.container.disk: 1234 > > > topology.container.ram: 3456 > > > topology.container.cpu: 2 > > > > > > > > > It may be best if I reused the ByteAmount object to calculate resource > > > size to remain consistent with the other Heron APIs. Any concerns or > > > improvements to this approach I am missing? > > > > > > Please Advise, > > > > > > Josh > > > > > >
