Bug in jepsen.nemesis.combined/compose-package

[stevana]

In jepsen.nemesis.combined/compose-packages when combining :final-generator the code looks like this:

   :final-generator (apply concat (keep :final-generator packages))

The apply concat is there to handle the db-generators case when there's a vector of generators. This code used to be apply gen/concat, which would work on both single generators, i.e. maps, and vector of generators. But now with the new code when concat is a applied to a map it results in broken generators:

(def opts {:faults #{:partition}
           :interval 10})
(:final-generator (combined/compose-packages [(combined/partition-package opts)]))
=>
([:type :info] [:f :stop-partition] [:value nil])

There are several ways to fix this. Me and @Danten are leaning towards only allowing final-generator to be a single generator, i.e. change db-generators to be :final-generator (gen/phases final) and remove apply concat from compose-packages.

Thoughts?

[aphyr]

Oh! Good point. We can use gen/concat, right?

[stevana]

Oh! Good point. We can use gen/concat, right?

I thought that function was removed now that sequences are generators out of the box?

[aphyr]

Nope! I left it in just in case we had a situation like this. Wasn't clever enough to see it in this case though!

[stevana]

Where did you hide it then? I can't find it when I search for concat in jepsen.generator.

[aphyr]

Or... Maybe I'm lying and there isn't one! I could have sworn I wrote a concat in generators.pure... must have taken it out. At the moment I'm, uh, well my partner just got out of surgery and is asleep partly resting on top of me so I can't move, but once I'm back to a computer I can go look. Or you can submit a PR for it.

[stevana]

The following seems to work:

(defn compose-packages
  "Takes a collection of nemesis+generators packages and combines them into
  one. Generators are mixed together randomly; final generators proceed
  sequentially."
  [packages]
  {:generator       (gen/mix (map :generator packages))
   :final-generator (apply concat
                           (keep
                            (fn [pkg]
                              (let [gen (:final-generator pkg)]
                                (if (seq? gen)
                                  gen
                                  [gen])))
                              packages))
   :nemesis         (n/compose (map :nemesis packages))
   :perf            (reduce into #{} (map :perf packages))})

But I guess it's more elegant to hide that logic inside gen/concat? I'm not sure I understand the generators well enough to do that though...

[aphyr]

I think it really could be as simple as (defn concat [& gens] gens), but lemme test first.

[aphyr]

Yeah, there DID used to be a concat gen, but I took it out before release--thought it was unnecessary with concat and the seq implementations. You can generally write [a b c] (or any sequence) instead of (concat a b c), but I've added an explicit concat generator to make that obvious, and just in case we want to mess with update propagation later.

I think in this particular case, we can drop the call to concat altogether, and just use (keep :final-generator packages).