Surprising behaviour of the new generators

[stevana]

The following test fails:

(deftest phases-clients-nemesis-test
  (is (= [[0        :op   nil]
          [:nemesis :kill ["n1"]]]
         (->> (gen/phases (->> {:f :op, :value nil}
                               (gen/clients))
                          (gen/nemesis {:type :info, :f :kill, :value ["n1"]}))
              ; gen/clients
              perfect
              (map (juxt :process :f :value))))))

The nemesis kill op isn't generated for some reason?

Also if we uncomment gen/clients above perfect, we get the following error: java.lang.AssertionError: Assert failed: generator pending and nothing in flight??? op. I guess gen/clients doesn't make sense there, but still this seems to be a change from the previous generators?

[aphyr]

Ah, you're bumping up against one of those Bad API Choices I talked about in the other ticket just now: perfect only returns invocations, so you can't use it to test nemeses. See perfect. I haven't bothered polishing these because they're only used for testing, but if they're being moved into jepsen.generator proper, it'd be worth doing a pass to make sure they have a unified, mostly-orthogonal design, whether that's the current perfect vs perfect or (invocations (perfect ...)) instead of (perfect ...).

[aphyr]

The gen/clients issue... I agree, it's confusing, but after some thought, I think I understand: you're asking the nemesis to do something, but only giving it clients: there's no way it can perform the kill you've asked for.

[aphyr]

There might be an argument that this is the wrong API choice, btw! It might be the case that we want (gen/clients) to work more like filter... might be worth investigating whether we can do The Obvious Thing and have gen/phases realize it shouldn't try to block on gen/nemesis.

[stevana]

What about this one:

(deftest looping-test
  (is (= [[0 :op nil]]
         (->> (gen/phases
               {:f :op, :value nil}
               gen/clients)
              perfect
              (map (juxt :process :f :value))))))

It seems to get stuck in a loop and not terminate. I think that the problem is that gen/clients needs to be applied to a generator, that it merely diverges doesn't seem like the best way to communicate that to the user though?

[aphyr]

This is technically a valid generator! gen/clients is a function, therefore a generator. It's being called with... a context map? (maps are generators, so that's legal) and spitting out a generator in return (as expected), which is asked for an op, and it probably emits that context map with friends. Unconventional, and I assume this is a typo on your part, but I don't think it's actually a bug. At least not from a type perspective. There's an argument, of course, that any program which doesn't do what you meant is buggy, and in that sense this is a bug. ;-)

--Kyle

[stevana]

The following also diverges:

(deftest clients-clients-test4
  (is (= [[0        :invoke :append nil]
          [0         :ok     :append nil]
          [1         :invoke :append nil]
          [1         :ok     :append nil]
          [1         :invoke :read 2]
          [1         :ok     :read 2]]
         (->> (gen/phases
               (gen/clients {:f :append, :value nil})
               (gen/clients {:f :append, :value nil})
               (gen/clients (fn [] {:f :read, :value 2})))
               ;; (gen/clients {:f :read, :value 2}))
              perfect*
              (map (juxt :process :type :f :value))))))

When the read generator is a map rather than a function as in the comment then it works.

[aphyr]

This is expected behavior: see the documentation for functions in the generator namespace.

[stevana]

Thanks for clearing up my misunderstandings! Here's one more that I got, I'm trying to keep some state around when generating:

(defn should-bump-count
  [event]
  (case (:type event)
    :ok (case (:f event)
          :inc true
          :read   false)
    false))

(defrecord MyGenerator [count]
  gen/Generator
  (op [_ test ctx]
    (gen/op (gen/phases {:f :inc, :value nil}
                        {:f :inc, :value nil}
                        {:f :read, :value count}) test ctx))
  (update [this _ _ event]
    (if (should-bump-count event)
      (MyGenerator. (inc count))
      this)))

(defn generator
  []
  (MyGenerator. 0))

(deftest clients-clients-test5
  (is (= [[0 :invoke :inc nil]
          [0 :ok :inc nil]
          [1 :invoke :inc nil]
          [1 :ok :inc nil]
          [1 :invoke :read 0]
          [1 :ok :read 0]]
         (->> (gen/clients (generator))
              perfect*
              (map (juxt :process :type :f :value))))))

This passes, while I was expecting read to return 2... Is it possible to keep state like this in a pure way? Or do I have to be dirty and use gen/on-update together with references (atom and swap!)?

[stevana]

I don't seem to get any events when I slap gen/on-update on top of my nemesis generators? The old trick of simply passing around a reference/atom and doing swap! in the generators seems to yield unexpected results (presumably generators are evaluated in unexpected orders/multiple times now that they are pure?). Doing it purely like I tried above doesn't seem to work either, which makes me wonder: how is one supposed to do stateful nemesis generators these days?

[aphyr]

Hi, just got back from backpacking. As the documentation notes, yes: these generators are pure, and you should expect them to be called speculatively--while you can use mutable state, the execution semantics are gonna be complicated.The reason your code sample two posts up fails is because you're creating a fresh generator for every invocation of (op) and asking it for an operation; you probably didn't mean to do this.--On Jul 16, 2020 09:43, Stevan Andjelkovic notifications@github.com wrote: I don't seem to get any events when I slap gen/on-update on top of my nemesis generators? The old trick of simply passing around a reference/atom and doing swap! in the generators seems to yield unexpected results (presumably generators are evaluated in unexpected orders/multiple times now that they are pure?). Doing it purely like I tried above doesn't seem to work either, which makes me wonder: how is one supposed to do stateful nemesis generators these days?

—You are receiving this because you commented.Reply to this email directly, view it on GitHub, or unsubscribe.

[stevana]

Thanks for all the help! (Another thing that bit me was using on-update inside a mix...)

[aphyr]

Ah, on-update within mix is a good candidate for surprise--it's that way to prevent some spectacular performance regressions, but there's a solid argument for a version of mix which takes, say, an option map as its first argument, and one option could be to propagate updates.

[stevana]

Another surprise: (gen/on-update f (gen/on-update g gen)) is equivalent to gen/on-update f gen rather than gen/on-update (comp f g) gen.

Is there a reason why you didn't go with the latter?

[aphyr]

You can open new issues for these, BTW!

This is right in the documentation for on-update, so I'm not sure why it's surprising... You want the ability to intercept updates and have the choice of whether, and how, to propagate them to downstream generators. (comp f g) wouldn't make sense from a type perspective anyway--they're four-arity functions, not unary, and g returns a generator--it wouldn't make sense to apply it to f.

[aphyr]

Are you sure that (on-update f (on-update g gen)) is equivalent to (on-update f gen), or is this... potentially an artifact of your choice of f (e.g. does f not propagate updates to the downstream generator?) If this were true, it'd be a pretty big bug, but... looking at the code, I don't see how that can happen just yet. Got a test case?

[stevana]

I'm not saying there's a bug in the implementation or documentation of on-update, I'm merely surprised by your design choice. The following code passes, and illustrates how (on-update f (on-update g gen)) is equivalent to (on-update f gen):

(def gen-state (atom 0))

(defn transition-invoke
  [gen _test _ctx event]
  (case (:type event)
    :invoke (case (:f event)
              :append (do (swap! gen-state inc)
                          gen)
              :read   gen)
    gen))

(defn transition-ok
  [gen _test _ctx event]
  (case (:type event)
    :ok (case (:f event)
          :append (do (swap! gen-state inc)
                      gen)
          :read   gen)
    gen))

(deftest update-update-test
  (is (= [[0         :invoke :append nil]
          [0         :ok     :append nil]
          [1         :invoke :append nil]
          [1         :ok     :append nil]
          [1         :invoke :read 2]
          [1         :ok     :read 2]]
         (->> (gen/on-update
               transition-invoke
               (gen/on-update
                transition-ok
                (gen/phases
                 (gen/clients {:f :append, :value nil})
                 (gen/clients {:f :append, :value nil})
                 (gen/clients
                  (gen/once
                   (fn [] {:f :read, :value @gen-state}))))))
              (gen/limit 10)
              gen.test/perfect*
              (map (juxt :process :type :f :value))))))

To me it seems less surprising that both transition-ok and transition-invoke should be run and we get :read 4 above, i.e. that (on-update f (on-update g gen)) is equivalent to on-update (fn [gen test ctx event] (f (g gen test ctx event) test ctx event) gen (which I sloppily wrote as on-update (comp f g) gen before). The following variant of on-update achieves this:

(defrecord OnUpdate2 [f gen]
  gen/Generator
  (op [this test ctx]
    (when-let [[op gen'] (gen/op gen test ctx)]
      [op (OnUpdate2. f gen')]))

  (update [this test ctx event]
    (f (OnUpdate2. f (gen/update gen test ctx event)) test ctx event)))

(defn on-update2
  [f gen]
  (OnUpdate2. f gen))

(deftest update-update-test2
  (is (= [[0         :invoke :append nil]
          [0         :ok     :append nil]
          [1         :invoke :append nil]
          [1         :ok     :append nil]
          [1         :invoke :read 4]
          [1         :ok     :read 4]]
         (->> (on-update2
               transition-invoke
               (on-update2
                transition-ok
                (gen/phases
                 (gen/clients {:f :append, :value nil})
                 (gen/clients {:f :append, :value nil})
                 (gen/clients
                  (gen/once
                   (fn [] {:f :read, :value @gen-state}))))))
              (gen/limit 10)
              gen.test/perfect*
              (map (juxt :process :type :f :value))))))

I was merely wondering why you didn't go with this variant instead given that it composes.

[aphyr]

This is because you've chosen explicitly not to propagate the update event to the downstream generator in transition-invoke: that function returns the nested gen unchanged, instead of, say, calling (gen/update gen test ctx event).

[stevana]

Oh, I didn't realise you could solve it that way as well. Makes sense now, cheers!

[aphyr]

Yep! And the reason it works this way--I alluded to this above, but to make it concrete--is that if you did what you were thinking for OnUpdate, it wouldn't let you control how the underlying generator evolved. OnUpdate gives you the option to ignore updates, to transform them, to add additional context, to replace itself with some simpler generator once some condition has arisen, etc etc. In exchange for that generality, we have to explicitly say "Return my underlying generator with this update applied as well".

For instance, you might want to have a generator that produces writes until a certain number of writes have completed ok, then performs a final read. Haven't actually tried this, but it probably goes something like:

(on-update (fn [this test ctx event]
  (if (and (= :write (:f event)) (= :ok (:type event)))
    (let [write-count (inc (:write-count this 0))]
      (if (<= 10 write-count)
        ; We're done; replace this generator with a single read
        {:f :read}
        ; Still waiting; record the new write count and propagate the update to our generator.
        (assoc this :write-count write-count
                    :gen         (gen/update (:gen this) test ctx event)))))
    ; Something other than a successful write
    (assoc this :gen (gen/update (:gen this) test ctx event)))))

[stevana]

Gotcha, and thanks for the example!