Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

part: Fix MarshalYAML on empty maps #67

Merged
merged 1 commit into from
Nov 8, 2024

Conversation

joamaki
Copy link
Contributor

@joamaki joamaki commented Nov 7, 2024

MarshalYAML nil deref'd the 'm.tree' when the map was empty. Fix this and extend test coverage.

MarshalYAML nil deref'd the 'm.tree' when the map was empty.
Fix this and extend test coverage.

Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>
@joamaki joamaki requested a review from a team as a code owner November 7, 2024 15:56
@joamaki joamaki requested review from ovidiutirla and removed request for a team November 7, 2024 15:56
Copy link

github-actions bot commented Nov 7, 2024

$ make test
            ID   Faulty   StatusKind   StatusError
            1    false    Pending      
            2    false    Done         
            3    false    Done         
            > db cmp test-objects step1+3.table
        scripttest.go:230: FAIL: testdata/incremental.txtar:27: db cmp test-objects step1+3.table: table mismatch:
              ID  StatusKind StatusError
            - 1   Error      update fail
            + 1   Done
              2   Done       
              3   Done       
        logger.go:257: time=2024-11-07T15:57:34.301Z level=INFO source=/home/runner/go/pkg/mod/github.com/cilium/hive@v0.0.0-20241025140746-d66ad09f4384/hive.go:354 msg=Stopping
        logger.go:257: time=2024-11-07T15:57:34.301Z level=INFO source=/home/runner/go/pkg/mod/github.com/cilium/hive@v0.0.0-20241025140746-d66ad09f4384/cell/lifecycle.go:172 msg="Stop hook executed" function=*job.group.Stop duration=22.973µs
        logger.go:257: time=2024-11-07T15:57:34.301Z level=INFO source=/home/runner/go/pkg/mod/github.com/cilium/hive@v0.0.0-20241025140746-d66ad09f4384/cell/lifecycle.go:172 msg="Stop hook executed" function="reconciler_test.newEngine.func4.1 (.../statedb/reconciler/script_test.go:81) (test)" duration=56.805µs
        logger.go:257: time=2024-11-07T15:57:34.301Z level=INFO source=/home/runner/go/pkg/mod/github.com/cilium/hive@v0.0.0-20241025140746-d66ad09f4384/cell/lifecycle.go:172 msg="Stop hook executed" function="statedb.newHiveDB.func2 (.../statedb/statedb/cell.go:37) (statedb)" duration=4.127µs
FAIL
coverage: 89.4% of statements
FAIL	github.com/cilium/statedb/reconciler	1.107s
FAIL
make: *** [Makefile:9: test] Error 1
-----
$ 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                    	  435471	      2648 ns/op	    377637 objects/sec	    2816 B/op	      32 allocs/op
BenchmarkDB_WriteTxn_10-4                   	 1221579	       977.8 ns/op	   1022721 objects/sec	     742 B/op	      10 allocs/op
BenchmarkDB_WriteTxn_100-4                  	 1543834	       776.8 ns/op	   1287316 objects/sec	     599 B/op	       7 allocs/op
BenchmarkDB_WriteTxn_1000-4                 	 1501730	       800.1 ns/op	   1249839 objects/sec	     549 B/op	       7 allocs/op
BenchmarkDB_WriteTxn_100_SecondaryIndex-4   	  500737	      2464 ns/op	    405869 objects/sec	    1504 B/op	      37 allocs/op
BenchmarkDB_Modify-4                        	    1300	    926659 ns/op	   1079147 objects/sec	  773204 B/op	    8462 allocs/op
BenchmarkDB_GetInsert-4                     	    1225	   1017088 ns/op	    983201 objects/sec	  762074 B/op	    8468 allocs/op
BenchmarkDB_RandomInsert-4                  	    2329	    504083 ns/op	   1983801 objects/sec	  402242 B/op	    7098 allocs/op
BenchmarkDB_RandomReplace-4                 	     314	   3793415 ns/op	    263615 objects/sec	 2354248 B/op	   48569 allocs/op
BenchmarkDB_SequentialInsert-4              	    1494	    802702 ns/op	   1245792 objects/sec	  549574 B/op	    7285 allocs/op
BenchmarkDB_Changes_Baseline-4              	    1267	    947199 ns/op	   1055746 objects/sec	  553169 B/op	   10252 allocs/op
BenchmarkDB_Changes-4                       	     703	   1707682 ns/op	    585589 objects/sec	  993456 B/op	   14493 allocs/op
BenchmarkDB_RandomLookup-4                  	   20779	     57268 ns/op	  17461804 objects/sec	     160 B/op	       1 allocs/op
BenchmarkDB_SequentialLookup-4              	   27162	     44028 ns/op	  22712900 objects/sec	       0 B/op	       0 allocs/op
BenchmarkDB_Prefix_SecondaryIndex-4         	    7626	    147795 ns/op	   6766142 objects/sec	   93896 B/op	    1044 allocs/op
BenchmarkDB_FullIteration_All-4             	     723	   1614720 ns/op	  61930349 objects/sec	     480 B/op	      12 allocs/op
BenchmarkDB_FullIteration_Get-4             	     208	   5872254 ns/op	  17029265 objects/sec	       0 B/op	       0 allocs/op
BenchmarkDB_PropagationDelay-4              	  504415	      2358 ns/op	        20.00 50th_µs	        24.00 90th_µs	       107.0 99th_µs	    1589 B/op	      24 allocs/op
PASS
ok  	github.com/cilium/statedb	27.945s
PASS
ok  	github.com/cilium/statedb/index	0.004s
PASS
ok  	github.com/cilium/statedb/internal	0.004s
goos: linux
goarch: amd64
pkg: github.com/cilium/statedb/part
cpu: AMD EPYC 7763 64-Core Processor                
Benchmark_Insert_RootOnlyWatch-4    	    8383	    134399 ns/op	   7440555 objects/sec	  104165 B/op	    2041 allocs/op
Benchmark_Insert-4                  	    6007	    182033 ns/op	   5493513 objects/sec	  219064 B/op	    3064 allocs/op
Benchmark_Modify-4                  	    8048	    144953 ns/op	   6898821 objects/sec	  212534 B/op	    1205 allocs/op
Benchmark_GetInsert-4               	    6782	    173410 ns/op	   5766687 objects/sec	  212551 B/op	    1205 allocs/op
Benchmark_Replace-4                 	26683308	        44.54 ns/op	  22453274 objects/sec	       0 B/op	       0 allocs/op
Benchmark_Replace_RootOnlyWatch-4   	26986744	        44.62 ns/op	  22413410 objects/sec	       0 B/op	       0 allocs/op
Benchmark_txn_1-4                   	 3040584	       394.0 ns/op	   2538325 objects/sec	     448 B/op	       7 allocs/op
Benchmark_txn_10-4                  	 7472359	       159.6 ns/op	   6263791 objects/sec	     154 B/op	       2 allocs/op
Benchmark_txn_100-4                 	 8203796	       150.1 ns/op	   6661165 objects/sec	     224 B/op	       2 allocs/op
Benchmark_txn_1000-4                	 7271566	       166.3 ns/op	   6011877 objects/sec	     216 B/op	       2 allocs/op
Benchmark_txn_delete_1-4            	 3169220	       378.2 ns/op	   2644354 objects/sec	     856 B/op	       6 allocs/op
Benchmark_txn_delete_10-4           	 8381287	       143.4 ns/op	   6973469 objects/sec	     132 B/op	       1 allocs/op
Benchmark_txn_delete_100-4          	10206634	       116.4 ns/op	   8590661 objects/sec	      60 B/op	       1 allocs/op
Benchmark_txn_delete_1000-4         	10925403	       110.6 ns/op	   9039694 objects/sec	      26 B/op	       1 allocs/op
Benchmark_Get-4                     	   37924	     32048 ns/op	  31203689 objects/sec	       0 B/op	       0 allocs/op
Benchmark_Iterate-4                 	  169982	      6800 ns/op	 147062076 objects/sec	      80 B/op	       3 allocs/op
Benchmark_Hashmap_Insert-4          	   15534	     76827 ns/op	  13016234 objects/sec	   86553 B/op	      64 allocs/op
Benchmark_Hashmap_Get_Uint64-4      	  154748	      7762 ns/op	 128840354 objects/sec	       0 B/op	       0 allocs/op
Benchmark_Hashmap_Get_Bytes-4       	  147288	      8095 ns/op	 123529550 objects/sec	       0 B/op	       0 allocs/op
Benchmark_Uint64Map_Random-4        	    1341	    888705 ns/op	   1125234 items/sec	 2701401 B/op	    9031 allocs/op
Benchmark_Uint64Map_Sequential-4    	    1436	    839747 ns/op	   1190836 items/sec	 2492406 B/op	    9749 allocs/op
PASS
ok  	github.com/cilium/statedb/part	28.302s
PASS
ok  	github.com/cilium/statedb/reconciler	0.005s
?   	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 3.03 seconds (batch size 1000)
Throughput 329804.99 objects per second
Allocated 6011303 objects, 424777kB bytes, 514568kB bytes still in use

@joamaki
Copy link
Contributor Author

joamaki commented Nov 7, 2024

Test failing due to #66. And it looks like I've made the test pass even when it fails in the PR that changed the workflow to post the message even when it fails. Oops. Need to look into that..

Copy link

@ovidiutirla ovidiutirla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

part/map.go Show resolved Hide resolved
@joamaki joamaki merged commit ce7e907 into main Nov 8, 2024
1 check passed
@joamaki joamaki deleted the pr/joamaki/fix-part-empty-map-yaml branch November 8, 2024 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants