API should not allow mixins between VariableResources and FixedResources

[clintval]

The API allows you to mix in both VariableResources and FixedResources into the same task when I think the type checker or compiler should forbid the combination.

For example, this compiles:

class MyAction(val minThreads: Int, val maxThreads: Int)
  extends ProcessTask
  with Pipe[Text, Text]
  with FixedResources
  with VariableResources {

  override def pickResources(availableResources: ResourceSet): Option[ResourceSet] = {
    resources.subset(Cores(minThreads), Cores(maxThreads), Memory("1G"))
  }

  override def args: Seq[Any] = Seq.empty
}

When the above task is scheduled on its own, the naive scheduler will call the pick resources function and the task will behave as a variable resource task. Counter-intuitively, it will be scheduled under the fixed resources default when wrapped in a pipechain because we partition tasks in a pipe chain based on whether they implement FixedResources or not.

That was an obvious example.

But here is one that bit me:

class MyAction(val minThreads: Int, val maxThreads: Int)
  extends ProcessTask
  with Pipe[Text, Text]
  with MemoryDoublingRetry
  with VariableResources {

  override def pickResources(availableResources: ResourceSet): Option[ResourceSet] = {
    resources.subset(Cores(minThreads), Cores(maxThreads), Memory("1G"))
  }

  override def args: Seq[Any] = Seq.empty
}

Because MemoryDoublingRetry <: MemoryRetry <: FixedResources.

[nh13]

@clintval I think making pickResources final in FixedResources solves them both. What do you think?

[clintval]

I looked into this briefly.

There are four classes that override pickResources that mixin FixedResources so those tasks would have to change. It's possible other users are depending on being able to override that method as well.

• CallDuplexConsensusReads
• CollectVariantCallingMetrics
• ExtractIlluminaBarcodes
• IlluminaBasecallsToSam

I didn't see any easy way out on this one as introducing final would certainly break things. I'm cool leaving this issue open as a warning for anyone else that stumbles across this oddity!

[nh13]

I'm ok with breaking things to correct things, what do you think @tfenne ?