Reimplement script commands against hive script #57

joamaki · 2024-10-01T13:22:46Z

Reimplement the script commands with the hive/script library. And with these reimplement the reconciler tests. I tried to be faithful to the original test cases, but please double-check.

See testdata/db.txtar for an overview of the commands.

I'll do a separate pass to add documentation to the README here and to the cilium docs once we've gotten a bit more experience with this.

❯ go run .
time=2024-10-09T15:10:36.412+02:00 level=INFO msg=Invoked duration=121.02µs function="main.writeStuff (.../statedb/scriptexample/main.go:75)"
> hive start
time=2024-10-09T15:10:37.713+02:00 level=INFO msg=Starting
time=2024-10-09T15:10:37.713+02:00 level=INFO msg="Start hook executed" function="statedb.newHiveDB.func1 (.../cilium/statedb/cell.go:34) (statedb)" duration=74.336µs
time=2024-10-09T15:10:37.713+02:00 level=INFO msg="Start hook executed" function="*job.group.Start (example)" duration=14.949µs
time=2024-10-09T15:10:37.713+02:00 level=INFO msg=Started duration=502.237µs
> db tables
Name   Object count   Deleted objects   Indexes   Initializers   Go type      Last WriteTxn
ex     3              0                 id, x     []             main.ExObj   example (87.0ms ago, locked for 148.4us)
> db show ex
[stdout]
Name    X
count   3
one     1
two     2
> db list ex one
[stdout]
Name   X
one    1
> db prefix ex o
[stdout]
Name   X
one    1
> db
<stdin>:0: db: expected command (show, cmp, delete, prefix, lowerbound, tables, insert, get, list, initialized)
> db initialized
ex initialized
> db prefix -delete ex o
Deleted 1 objects
[stdout]
...

github-actions · 2024-10-01T13:24:20Z

$ make test
go: downloading github.com/mitchellh/mapstructure v1.5.0
go: downloading golang.org/x/tools v0.17.0
go: downloading golang.org/x/sys v0.17.0
go: downloading github.com/spf13/cast v1.6.0
go: downloading github.com/fsnotify/fsnotify v1.7.0
go: downloading github.com/sagikazarmark/slog-shim v0.1.0
go: downloading github.com/spf13/afero v1.11.0
go: downloading golang.org/x/text v0.14.0
go: downloading github.com/subosito/gotenv v1.6.0
go: downloading github.com/hashicorp/hcl v1.0.0
go: downloading gopkg.in/ini.v1 v1.67.0
go: downloading github.com/magiconair/properties v1.8.7
go: downloading github.com/pelletier/go-toml/v2 v2.1.0
	github.com/cilium/statedb/reconciler/benchmark		coverage: 0.0% of statements
	github.com/cilium/statedb/reconciler/example		coverage: 0.0% of statements
ok  	github.com/cilium/statedb	26.603s	coverage: 85.9% of statements
ok  	github.com/cilium/statedb/index	0.005s	coverage: 28.7% of statements
ok  	github.com/cilium/statedb/internal	0.008s	coverage: 46.7% of statements
ok  	github.com/cilium/statedb/part	2.799s	coverage: 82.9% of statements
ok  	github.com/cilium/statedb/reconciler	0.282s	coverage: 88.9% of statements
-----
$ make bench
go test ./... -bench . -benchmem -test.run xxx
goos: linux
goarch: amd64
pkg: github.com/cilium/statedb
cpu: AMD EPYC 7763 64-Core Processor                
BenchmarkDB_WriteTxn_1-4                    	  424218	      2689 ns/op	    371824 objects/sec	    2816 B/op	      32 allocs/op
BenchmarkDB_WriteTxn_10-4                   	 1226389	       973.3 ns/op	   1027452 objects/sec	     743 B/op	      10 allocs/op
BenchmarkDB_WriteTxn_100-4                  	 1556047	       770.9 ns/op	   1297228 objects/sec	     602 B/op	       7 allocs/op
BenchmarkDB_WriteTxn_1000-4                 	 1499061	       800.2 ns/op	   1249727 objects/sec	     551 B/op	       7 allocs/op
BenchmarkDB_WriteTxn_100_SecondaryIndex-4   	  506275	      2305 ns/op	    433885 objects/sec	    1502 B/op	      37 allocs/op
BenchmarkDB_Modify-4                        	    1318	    984978 ns/op	   1015252 objects/sec	  773441 B/op	    8462 allocs/op
BenchmarkDB_GetInsert-4                     	    1212	   1009967 ns/op	    990133 objects/sec	  763646 B/op	    8469 allocs/op
BenchmarkDB_RandomInsert-4                  	    2414	    498045 ns/op	   2007851 objects/sec	  402517 B/op	    7099 allocs/op
BenchmarkDB_RandomReplace-4                 	     316	   3755806 ns/op	    266254 objects/sec	 2358469 B/op	   48570 allocs/op
BenchmarkDB_SequentialInsert-4              	    1518	    800785 ns/op	   1248774 objects/sec	  551168 B/op	    7286 allocs/op
BenchmarkDB_Changes_Baseline-4              	    1270	    953313 ns/op	   1048974 objects/sec	  553166 B/op	   10252 allocs/op
BenchmarkDB_Changes-4                       	     704	   1688692 ns/op	    592174 objects/sec	  993147 B/op	   14493 allocs/op
BenchmarkDB_RandomLookup-4                  	   21306	     54424 ns/op	  18374366 objects/sec	     160 B/op	       1 allocs/op
BenchmarkDB_SequentialLookup-4              	   26338	     45517 ns/op	  21969974 objects/sec	       0 B/op	       0 allocs/op
BenchmarkDB_Prefix_SecondaryIndex-4         	    7713	    146043 ns/op	   6847326 objects/sec	   93898 B/op	    1044 allocs/op
BenchmarkDB_FullIteration_All-4             	     890	   1393075 ns/op	  71783763 objects/sec	     480 B/op	      12 allocs/op
BenchmarkDB_FullIteration_Get-4             	     214	   5947407 ns/op	  16814076 objects/sec	       0 B/op	       0 allocs/op
BenchmarkDB_PropagationDelay-4              	  513145	      2321 ns/op	        20.00 50th_µs	        23.00 90th_µs	       100.0 99th_µs	    1589 B/op	      24 allocs/op
PASS
ok  	github.com/cilium/statedb	28.016s
PASS
ok  	github.com/cilium/statedb/index	0.003s
PASS
ok  	github.com/cilium/statedb/internal	0.003s
goos: linux
goarch: amd64
pkg: github.com/cilium/statedb/part
cpu: AMD EPYC 7763 64-Core Processor                
Benchmark_Insert_RootOnlyWatch-4    	    9213	    132728 ns/op	   7534226 objects/sec	  104161 B/op	    2041 allocs/op
Benchmark_Insert-4                  	    6432	    181777 ns/op	   5501248 objects/sec	  219075 B/op	    3065 allocs/op
Benchmark_Modify-4                  	    8272	    143050 ns/op	   6990572 objects/sec	  212356 B/op	    1204 allocs/op
Benchmark_GetInsert-4               	    6516	    172724 ns/op	   5789587 objects/sec	  212302 B/op	    1203 allocs/op
Benchmark_Replace-4                 	27117450	        44.97 ns/op	  22237358 objects/sec	       0 B/op	       0 allocs/op
Benchmark_Replace_RootOnlyWatch-4   	26969168	        44.64 ns/op	  22400006 objects/sec	       0 B/op	       0 allocs/op
Benchmark_txn_1-4                   	 3037495	       394.0 ns/op	   2537957 objects/sec	     448 B/op	       7 allocs/op
Benchmark_txn_10-4                  	 7578582	       159.4 ns/op	   6272964 objects/sec	     154 B/op	       2 allocs/op
Benchmark_txn_100-4                 	 8268332	       143.5 ns/op	   6969645 objects/sec	     223 B/op	       2 allocs/op
Benchmark_txn_1000-4                	 7355799	       162.6 ns/op	   6151880 objects/sec	     216 B/op	       2 allocs/op
Benchmark_txn_delete_1-4            	 3172098	       377.4 ns/op	   2649624 objects/sec	     856 B/op	       6 allocs/op
Benchmark_txn_delete_10-4           	 8384698	       142.4 ns/op	   7024376 objects/sec	     132 B/op	       1 allocs/op
Benchmark_txn_delete_100-4          	10261430	       114.3 ns/op	   8745844 objects/sec	      60 B/op	       1 allocs/op
Benchmark_txn_delete_1000-4         	11131402	       108.1 ns/op	   9251556 objects/sec	      26 B/op	       1 allocs/op
Benchmark_Get-4                     	   37834	     30795 ns/op	  32472573 objects/sec	       0 B/op	       0 allocs/op
Benchmark_Iterate-4                 	  173164	      6980 ns/op	 143260437 objects/sec	      80 B/op	       3 allocs/op
Benchmark_Hashmap_Insert-4          	   15854	     75357 ns/op	  13270167 objects/sec	   86558 B/op	      64 allocs/op
Benchmark_Hashmap_Get_Uint64-4      	  153115	      7752 ns/op	 128993760 objects/sec	       0 B/op	       0 allocs/op
Benchmark_Hashmap_Get_Bytes-4       	  148142	      8037 ns/op	 124422742 objects/sec	       0 B/op	       0 allocs/op
Benchmark_Uint64Map_Random-4        	    1376	    862559 ns/op	   1159341 items/sec	 2702093 B/op	    9030 allocs/op
Benchmark_Uint64Map_Sequential-4    	    1489	    796798 ns/op	   1255024 items/sec	 2492407 B/op	    9749 allocs/op
PASS
ok  	github.com/cilium/statedb/part	29.267s
PASS
ok  	github.com/cilium/statedb/reconciler	0.004s
?   	github.com/cilium/statedb/reconciler/benchmark	[no test files]
?   	github.com/cilium/statedb/reconciler/example	[no test files]
go run ./reconciler/benchmark -quiet
1000000 objects reconciled in 2.85 seconds (batch size 1000)
Throughput 350752.55 objects per second
Allocated 6011302 objects, 424767kB bytes, 531656kB bytes still in use

script.go

Bring in the new hive version that includes the script support. Fix the API usage. Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>

table.go

joamaki · 2024-10-09T14:02:25Z

index/string.go

@@ -12,6 +12,10 @@ func String(s string) Key {
 	return []byte(s)
 }

+func FromString(s string) (Key, error) {


I think it makes sense to have also these utility functions here for the FromString index field. Not sure what the naming convention should be. Maybe index.Int32String...

This also makes me wonder how should we think about e.g. prefix searching index that has index.Int32 as the key (e.g. big-endian encoded). If one calls index.Int32String with 65536 (0x0001_0000), should it return []byte{0x00, 0x01} and you'd get all objects with key 0x0001_0000 to 0x0001_FFFF...
This sort of thing isn't currently supported with the "fixed-sized" keys created with index.Int* etc.
If we would like to support it we'd have to have a different method for constructing prefix search keys.
I guess this is something we can visit in the future when a use-case arises for this.

Added the *String variants in the last commit, e.g. index.Int32String and so on. Kept index.FromString as index.StringString would've been weird. Or should I add it for consistency?

Or should I add it for consistency?

As a personal preference I'd go for index.StringString to be consistent, but agree that it is not a nice name 😓
No strong opinion on this, in the end both are fine for me.

joamaki · 2024-10-09T14:03:12Z

part/set.go

@@ -208,6 +211,32 @@ func (s *Set[T]) UnmarshalJSON(data []byte) error {
 	return nil
 }

+func (s Set[T]) MarshalYAML() (any, error) {


Added this for use in testdata/db.txtar as the test objects used Set[string] for tags.
Realized I need the yaml support for part.Map as well. And these need tests.

Done and moved to its own commit.

joamaki · 2024-10-09T14:06:29Z

types.go

+
+	// FromString is an optional conversion from string to a raw key.
+	// If implemented allows script commands to query with this index.
+	FromString func(key string) (index.Key, error)


I wonder if we want to make this mandatory or not. Would be a breaking change if we require it.
Might also make sense to have constructors for Index instead of having to fill in the struct.
I'm inclined to not make this mandatory as of yet as I want these changes to be non-breaking for now.

joamaki · 2024-10-09T14:09:14Z

script.go

+		return fmt.Errorf("-columns not supported with non-table formats")
+	}
+	switch format {
+	case "yaml":


Kind of tempted to pull in https://github.com/itchyny/gojq into this to allow assertions with a jq query. Otoh, it's a pretty big dependency so perhaps best to keep this minimal until there's a real need.

joamaki · 2024-10-09T14:11:04Z

script.go

+	case "table":
+		header := tbl.TableHeader()
+		if header == nil {
+			return fmt.Errorf("objects in table %q not TableWritable", tbl.Meta.Name())


Here's perhaps a good reason to change NewTable[Obj any] into NewTable[Obj TableWritable] (or maybe make a TableObject constraint). Again don't want to make this into a breaking change yet, but maybe for statedb v0.4...

To allow using part.Set[] and part.Map[] types with the YAML marshalling/unmarshalling in the StateDB script commands, add the custom YAML marshalling and unmarshalling methods for these types. Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>

pippolo84

Did a first pass and left two minor nits, gonna do another pass later focusing on reconciler tests.

script.go

Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>

Reimplement the reconciler tests using scripttest. This significantly simplifies the test-suite and allows easier verification of more complex scenarios. To allow for Status JSON and YAML marshalling, define custom UnmarshalJSON and UnmarshalYAML that also fill in the 'id'. The 'id' is used with StatusSet to efficiently allow multiple reconcilers to manipulate the status without conflicts, e.g. if the object status id is the same it can still be updated with the reconciliation result even if the object conflicted due to other reconciler's update to its status. Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>

Add *String counterparts (Int32 => Int32String) for implementing FromString in Index[]. Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>

pippolo84

This is great! 🚀

To the best of my knowledge, the new script-based tests seem reasonably equivalent to the old ones 👍
Tried also to stress the new test and it behaves correctly after more than 10k runs. 🎉

joamaki mentioned this pull request Oct 1, 2024

Hive script cilium/hive#17

Merged

joamaki force-pushed the pr/joamaki/hivescript branch from 0d30ab4 to 1724458 Compare October 1, 2024 14:13

joamaki commented Oct 2, 2024

View reviewed changes

script.go Outdated Show resolved Hide resolved

joamaki commented Oct 2, 2024

View reviewed changes

script.go Outdated Show resolved Hide resolved

joamaki commented Oct 2, 2024

View reviewed changes

script.go Outdated Show resolved Hide resolved

joamaki force-pushed the pr/joamaki/hivescript branch 11 times, most recently from dea1492 to d1d27a4 Compare October 9, 2024 12:50

go.mod: Bump hive dependency

7cf0cdf

Bring in the new hive version that includes the script support. Fix the API usage. Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>

joamaki force-pushed the pr/joamaki/hivescript branch 2 times, most recently from 35f2d78 to 27423b5 Compare October 9, 2024 12:56

joamaki changed the title ~~DRAFT: Reimplement script commands against hive script~~ Reimplement script commands against hive script Oct 9, 2024

joamaki marked this pull request as ready for review October 9, 2024 13:08

joamaki requested a review from a team as a code owner October 9, 2024 13:08

joamaki requested review from pippolo84 and removed request for a team October 9, 2024 13:08

joamaki force-pushed the pr/joamaki/hivescript branch from 27423b5 to f01b461 Compare October 9, 2024 13:18

joamaki commented Oct 9, 2024

View reviewed changes

table.go Outdated Show resolved Hide resolved

joamaki force-pushed the pr/joamaki/hivescript branch 3 times, most recently from d73ca35 to 950e499 Compare October 9, 2024 13:49

joamaki commented Oct 9, 2024

View reviewed changes

joamaki force-pushed the pr/joamaki/hivescript branch from 950e499 to 204b1f3 Compare October 9, 2024 14:04

joamaki commented Oct 9, 2024

View reviewed changes

part: Add YAML support to Set and Map

e5cf37c

To allow using part.Set[] and part.Map[] types with the YAML marshalling/unmarshalling in the StateDB script commands, add the custom YAML marshalling and unmarshalling methods for these types. Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>

joamaki force-pushed the pr/joamaki/hivescript branch 2 times, most recently from 06dad38 to 58f01e1 Compare October 9, 2024 15:07

pippolo84 reviewed Oct 10, 2024

View reviewed changes

script.go Outdated Show resolved Hide resolved

script.go Show resolved Hide resolved

joamaki added 3 commits October 10, 2024 14:34

Reimplement script commands against hive script

3a7463e

Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>

index: Add *String functions

4a2e8f4

Add *String counterparts (Int32 => Int32String) for implementing FromString in Index[]. Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>

joamaki force-pushed the pr/joamaki/hivescript branch from 58f01e1 to 4a2e8f4 Compare October 10, 2024 12:34

pippolo84 approved these changes Oct 10, 2024

View reviewed changes

joamaki merged commit c6862eb into main Oct 10, 2024
1 check passed

joamaki deleted the pr/joamaki/hivescript branch October 10, 2024 14:24

joamaki mentioned this pull request Oct 11, 2024

Hive scripts and the cilium shell cilium/cilium#35154

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reimplement script commands against hive script #57

Reimplement script commands against hive script #57

joamaki commented Oct 1, 2024 •

edited

Loading

github-actions bot commented Oct 1, 2024 •

edited

Loading

joamaki Oct 9, 2024 •

edited

Loading

joamaki Oct 9, 2024 •

edited

Loading

pippolo84 Oct 10, 2024

joamaki Oct 9, 2024 •

edited

Loading

joamaki Oct 9, 2024 •

edited

Loading

joamaki Oct 9, 2024

joamaki Oct 9, 2024

joamaki Oct 9, 2024

pippolo84 left a comment

pippolo84 left a comment

Reimplement script commands against hive script #57

Reimplement script commands against hive script #57

Conversation

joamaki commented Oct 1, 2024 • edited Loading

github-actions bot commented Oct 1, 2024 • edited Loading

joamaki Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

joamaki Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

pippolo84 Oct 10, 2024

Choose a reason for hiding this comment

joamaki Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

joamaki Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

joamaki Oct 9, 2024

Choose a reason for hiding this comment

joamaki Oct 9, 2024

Choose a reason for hiding this comment

joamaki Oct 9, 2024

Choose a reason for hiding this comment

pippolo84 left a comment

Choose a reason for hiding this comment

pippolo84 left a comment

Choose a reason for hiding this comment

joamaki commented Oct 1, 2024 •

edited

Loading

github-actions bot commented Oct 1, 2024 •

edited

Loading

joamaki Oct 9, 2024 •

edited

Loading

joamaki Oct 9, 2024 •

edited

Loading

joamaki Oct 9, 2024 •

edited

Loading

joamaki Oct 9, 2024 •

edited

Loading