Skip to content

Commit

Permalink
perf(limit): refactor limit to use NarrowStateTransformation (#4514)
Browse files Browse the repository at this point in the history
* chore: modernize limit tests

The tests were written in the old style. Updating to use the `testcase`
keyword so I can run it through the debugger in isolation.

* perf(limit): refactor limit to use NarrowStateTransformation

* chore: add feature flag for NarrowTransformation limit

* chore: make fmt

* refactor: NewNarrowLimitTransformation is public (to share with tests)

* test: run standard limit tests against narrow and initial impls

* fix: empty table case was failing due to bad offset vs chunk len check

* fix: make sure tests have distinct names

* chore: make generate

* test: dupe TestProcess_Limit_MultiBuffer for narrow version (fails)

* fix: draw down `state.n` with each iteration, reset the offset

This change fixes the MultiBuffer test.

* refactor: more explicit empty input handling

Previously the chunk length check was double-duty, happening as part of
state management. Not sure that was even correct, and could have
introduced a bug for certain cases.

This moves the "empty" check to where we initialize the transformation
state, and short-circuits.

* chore: favor tr (not xform) for consistency

* chore: rephrase

* chore: modernize tickscript flux tests

* chore: make generate

* fix: fixup offset handling

* refactor: handle 0 len chunk and n=0 cases in same block

* chore: enable feature flag for fluxtest
  • Loading branch information
Owen Nelson authored Mar 3, 2022
1 parent 0f0c88f commit 59e6485
Show file tree
Hide file tree
Showing 14 changed files with 474 additions and 20,764 deletions.
1 change: 1 addition & 0 deletions execute/executetest/dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ var testFlags = map[string]interface{}{
"optimizeUnionTransformation": true,
"vectorizedMap": true,
"optimizeAggregateWindow": true,
"narrowTransformationLimit": true,
}

type TestFlagger map[string]interface{}
Expand Down
14 changes: 14 additions & 0 deletions internal/feature/flags.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions internal/feature/flags.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,9 @@
key: optimizeAggregateWindow
default: false
contact: Jonathan Sternberg

- name: Narrow Transformation Limit
description: Enable the NarrowStateTransformation implementation of limit
key: narrowTransformationLimit
default: false
contact: Owen Nelson
12 changes: 6 additions & 6 deletions libflux/go/libflux/buildinfo.gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,10 @@ var sourceHashes = map[string]string{
"stdlib/contrib/bonitoo-io/alerta/alerta.flux": "ec29116d3f190f5f73d4070b58c90dc7e6c2597e6b9f757bb24dd4832df198aa",
"stdlib/contrib/bonitoo-io/hex/hex.flux": "1947b7a7dceb43f6e688f9085a0354d07a58c9a9d9d53bd1f3a1bed1b25326cd",
"stdlib/contrib/bonitoo-io/servicenow/servicenow.flux": "be3f6aa2cb823d4a61e4b01c1b6445de3e5c8bc738a1498a23d2beda7df18554",
"stdlib/contrib/bonitoo-io/tickscript/alert_test.flux": "7b015c21f8975d64a2165df7349c1b2b950f217bee1ab5e899aa145e8d8cca8a",
"stdlib/contrib/bonitoo-io/tickscript/alert_with_topic_test.flux": "ca6c74e271a0b36ab92efc0350c4cc0d28ccd46526da67569e4909382bec8911",
"stdlib/contrib/bonitoo-io/tickscript/deadman_empty_test.flux": "8a0fde4f1db8b4a526221adcc0007dfcd94e823cda7e67e5d6ddb491b1375d1c",
"stdlib/contrib/bonitoo-io/tickscript/deadman_threshold_test.flux": "9ded8ba9c2c3958c2897f262beb634cad39b170cec260c9353fae3365ff46572",
"stdlib/contrib/bonitoo-io/tickscript/alert_test.flux": "e3751038830c09b1bcd9bbf0c97b8cbbae78115a34e8bd9785ddec8bfb494bd6",
"stdlib/contrib/bonitoo-io/tickscript/alert_with_topic_test.flux": "f730d7f49080b2e9fae4c1e6575d65a5b49a1ef8b72d938b727e4359cd39779a",
"stdlib/contrib/bonitoo-io/tickscript/deadman_empty_test.flux": "7d0baf3fc03c950c4533ee4dd504450c46997086a7fdaede62752d7021ce26aa",
"stdlib/contrib/bonitoo-io/tickscript/deadman_threshold_test.flux": "32aaac0ea2c5fcd62f071cd5885f3a199cf8cf4d527b7d2aede703a0928bba81",
"stdlib/contrib/bonitoo-io/tickscript/tickscript.flux": "a9b426381c971c7e3373de463cd2e6758c56a67db9cbbcf55d63f2971ebbf4e1",
"stdlib/contrib/bonitoo-io/victorops/victorops.flux": "b10d8277a670dd738c090a87a77a2e420be6e5ceb0468a8067357da05bb5d812",
"stdlib/contrib/bonitoo-io/zenoss/zenoss.flux": "842fab4c1f61fd2c8d24acfda6b5c1f893623f5c3a351180411900ff8b6510d6",
Expand Down Expand Up @@ -522,8 +522,8 @@ var sourceHashes = map[string]string{
"stdlib/universe/key_values_test.flux": "84782827831644a5061e8adaca23b9eb57689af545840ea9ea38935dabce4f0c",
"stdlib/universe/keys_test.flux": "f743290972af52184cf7b71c5957e6e262d78c74263718cd8bc332759b9e7d95",
"stdlib/universe/last_test.flux": "9856d1125e6976cbd04957e8e929c6f5bd09e2ffb984b0b6a77fbefcdbfd4b98",
"stdlib/universe/limit_offset_test.flux": "1150686a1bea106891afcf5218e473d99306fc4759730e48e7693f541ff29c24",
"stdlib/universe/limit_test.flux": "19149ce21d5970e639b5250ac35821f6351d70a9f17868800c10f5e293db7503",
"stdlib/universe/limit_offset_test.flux": "53b526a48050cd81921371c7cd71314e6fdff8444bc97f5b1acbe57355b15701",
"stdlib/universe/limit_test.flux": "e1576c6b8b28db8c018be38fc207505ee17c73f2a3a90dd57ed9b82c23419312",
"stdlib/universe/lowestAverage_test.flux": "3debee2f9c626a637b8842cf23d32f88ba1bf2bf98b53744e7c699a0c8f833cf",
"stdlib/universe/lowestCurrent_test.flux": "f9575bdb7e2ee3a37153b296d9e59fa69334e675f2de4f4023c0e7aec541aba2",
"stdlib/universe/lowestMin_test.flux": "389fa9ceb38d066ad18a8b21220d4b554d8c31d83d80f1694bc703a14f86a0b2",
Expand Down
8 changes: 6 additions & 2 deletions stdlib/contrib/bonitoo-io/tickscript/alert_test.flux
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,9 @@ tickscript_alert = (table=<-) =>
)
|> drop(columns: ["_time"])

test _tickscript_alert = () =>
({input: testing.loadStorage(csv: inData), want: testing.loadMem(csv: outData), fn: tickscript_alert})
testcase tickscript_alert {
want = testing.loadMem(csv: outData)
got = testing.loadStorage(csv: inData) |> tickscript_alert()

testing.diff(want: want, got: got) |> yield()
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,9 @@ tickscript_alert = (table=<-) =>
)
|> drop(columns: ["_time"])

test _tickscript_alert = () =>
({input: testing.loadStorage(csv: inData), want: testing.loadMem(csv: outData), fn: tickscript_alert})
testcase tickscript_alert_with_topic {
want = testing.loadMem(csv: outData)
got = testing.loadStorage(csv: inData) |> tickscript_alert()

testing.diff(want: want, got: got) |> yield()
}
8 changes: 6 additions & 2 deletions stdlib/contrib/bonitoo-io/tickscript/deadman_empty_test.flux
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,9 @@ tickscript_deadman = (table=<-) =>
|> drop(columns: ["details"])
|> drop(columns: ["_time"])

test _tickscript_deadman = () =>
({input: testing.loadStorage(csv: inData), want: testing.loadMem(csv: outData), fn: tickscript_deadman})
testcase tickscript_deadman_empty {
want = testing.loadMem(csv: outData)
got = testing.loadStorage(csv: inData) |> tickscript_deadman()

testing.diff(want: want, got: got) |> yield()
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,9 @@ tickscript_deadman = (table=<-) =>
|> drop(columns: ["details"])
|> drop(columns: ["_time"])

test _tickscript_deadman = () =>
({input: testing.loadStorage(csv: inData), want: testing.loadMem(csv: outData), fn: tickscript_deadman})
testcase tickscript_deadman_threshold {
want = testing.loadMem(csv: outData)
got = testing.loadStorage(csv: inData) |> tickscript_deadman()

testing.diff(want: want, got: got) |> yield()
}
Loading

0 comments on commit 59e6485

Please sign in to comment.