k8s-cluster-simulator icon indicating copy to clipboard operation
k8s-cluster-simulator copied to clipboard

Single Static Submitter

Open ordovicia opened this issue 6 years ago • 6 comments

Based on the discussion about the submitter registration interface (https://github.com/pfnet-research/k8s-cluster-simulator/pull/191#issuecomment-482040505, https://github.com/pfnet-research/k8s-cluster-simulator/pull/191#issuecomment-482046764), this PR ...

  • Modifies NewKubeSim*() functions to take a single submitter as an argument
    • One cannot add submitters directly to KubeSim after the instantiation
  • Provides CompositeSubmitter struct
    • Combines one or more submitters into a single submitter

In this PR, CompositeSubmitter doesn't handle dynamic submitter addition. We can support it in a later PR.

Closes #151

ordovicia avatar Apr 12 '19 06:04 ordovicia

Can we receive a slice of submitters in the arguments? I know the composite pattern can be used to implement multiple submitters but giving multiple submitters to a simulator is more intuitive to grasp the relations of them as I described in the chart of the blog. It’s easier to understand than the composite. When a scheduler has 2 submitters, I don’t think you think the scheduler has a submitter which has 2 submitters. What do you think?

dtaniwaki avatar Apr 12 '19 12:04 dtaniwaki

Let me think about it a little bit more. Maybe naming the composite pattern like SubmitterGroup May help.

dtaniwaki avatar Apr 12 '19 12:04 dtaniwaki

In the design of the CompositeSubmitter (or you can call it by another name), I put emphasis on separating the responsibility of doing complex handling of submitters from KubeSim. We can keep it simple by transferring the responsibility of handling multiple submitters from KubeSim to CompositeSubmitter.

The currently supported "complex handling of submitters" is the registration of multiple static submitters, but when it becomes necessary to handle more complex situations such as dynamic registration, implementing these functionalities to KubeSim would make it complicated and the maintenance should be a burden.

Another way of thinking is KubeSim itself handles the simple situation of "register multiple (static) submitters", as of the current design. However, users would be confused if there are multiple ways; Even if KubeSim accepts multiple submitters, users can still use CompositeSubmitter.

I agree that passing multiple submitters directly to KubeSim is more intuitive than using a composite submitter, but the unintuitiveness would be mitigated by providing a common composite submitter. Users can in practice register multiple submitters only by writing a few additional lines to wrap the submitters in CompositeSubmitter. I have just added a comment to buildSubmitter() in example/main.go.

ordovicia avatar Apr 14 '19 14:04 ordovicia

when it becomes necessary to handle more complex situations such as dynamic registration

Aren't we dropping this feature?

So, it's just as simple as below:

queue := queue.NewPriorityQueue()
submitters := []Submitter{buildSubmitter}
sched := buildScheduler() // see below
kubesim := kubesim.NewKubeSimFromConfigPathOrDie(configPath, queue, submitters, sched)

However, users would be confused if there are multiple ways; Even if KubeSim accepts multiple submitters, users can still use CompositeSubmitter.

It's totally up to users. The composite pattern is just an implementation of a submitter. However, I think most of them just use a slice of submitters because it's more intuitive and easy.

dtaniwaki avatar Apr 14 '19 14:04 dtaniwaki

when it becomes necessary to handle more complex situations such as dynamic registration

Aren't we dropping this feature?

Yes, with this PR we will drop this feature from KubeSim. I meant that if users do need the dynamic submitter registration, they can opt-in this feature by implementing a custom submitter, not by extending KubeSim.

ordovicia avatar Apr 15 '19 10:04 ordovicia

I'm not sure why do we want to drop multiple submitter interface?? From a user perspective, I think multiple submitters would be useful.

Even if we provide CompositeSubmitter interface, this means we introduce another burden to users.

Why should we have to introduce single submitter interface??

Would you mind explaining this??

everpeace avatar Apr 16 '19 02:04 everpeace