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

Nhse d32 nhskv.i17 #19

Merged
merged 19 commits into from
Mar 28, 2024
Merged

Nhse d32 nhskv.i17 #19

merged 19 commits into from
Mar 28, 2024

Conversation

martinsumner
Copy link

Changes:

  • Make timeouts result in 503.
  • Make global secondary index timeout configurable via riak.conf.
  • Make configurable change to encode 2i results using thoas not mochijson2.

@martinsumner
Copy link
Author

martinsumner commented Feb 16, 2024

Did a quick experiment taking the JSON library from erlang/otp#8111. There are some issues, as for obvious reasons this is not designed to be compatible with earlier versions. With a naive implementation, and some hacks to resolve compatibility, it was slower in an encode test than thoas/mochijson2 (though probably due to the hacks).

I think in this case the performance improvements of thoas are worth having now without waiting or back-porting OTP-native JSON.

@martinsumner
Copy link
Author

OpenRiak/riak_test#10

@martinsumner
Copy link
Author

Comparison charts for mochijson2/thoas performance.

Absolute:
JSONCompare_Absolute

Relative:
JSONCompare_Relative

src/riak_index.erl Show resolved Hide resolved
src/riak_kv_clusteraae_fsm.erl Outdated Show resolved Hide resolved
@martinsumner
Copy link
Author

Having fixed my use of the json encode in the OTP PR, this now appears to be notably faster than thoas - #20

On OTP 22:

Testing Implementation mochijson2
Result set of 1K in 0ms Result set of 2K in 1ms Result set of 3K in 1ms Result set of 5K in 2ms Result set of 8K in 4ms Total time 10ms
Result set of 13K in 9ms Result set of 21K in 19ms Result set of 34K in 21ms Result set of 55K in 38ms Total time 89ms
Result set of 100K in 91ms Result set of 200K in 191ms Result set of 300K in 230ms Result set of 500K in 358ms Total time 872ms
Result set of 1M in 899ms Result set of 2M in 2082ms Result set of 3M in 2373ms Result set of 5M in 3235ms Total time 8590ms
Result set of 8M in 8948ms Result set of 13M in 8785ms Total time 17733ms


Testing Implementation thoas
Result set of 1K in 0ms Result set of 2K in 0ms Result set of 3K in 1ms Result set of 5K in 2ms Result set of 8K in 3ms Total time 8ms
Result set of 13K in 8ms Result set of 21K in 12ms Result set of 34K in 19ms Result set of 55K in 26ms Total time 67ms
Result set of 100K in 68ms Result set of 200K in 122ms Result set of 300K in 187ms Result set of 500K in 254ms Total time 632ms
Result set of 1M in 828ms Result set of 2M in 1421ms Result set of 3M in 1787ms Result set of 5M in 3083ms Total time 7121ms
Result set of 8M in 7341ms Result set of 13M in 7962ms Total time 15303ms


Testing Implementation otp
Result set of 1K in 0ms Result set of 2K in 0ms Result set of 3K in 0ms Result set of 5K in 1ms Result set of 8K in 2ms Total time 6ms
Result set of 13K in 3ms Result set of 21K in 6ms Result set of 34K in 12ms Result set of 55K in 18ms Total time 41ms
Result set of 100K in 29ms Result set of 200K in 60ms Result set of 300K in 89ms Result set of 500K in 141ms Total time 320ms
Result set of 1M in 546ms Result set of 2M in 608ms Result set of 3M in 877ms Result set of 5M in 1982ms Total time 4015ms
Result set of 8M in 5968ms Result set of 13M in 6372ms Total time 12341ms

On OTP 25:

Testing Implementation mochijson2
Result set of 1K in 0ms Result set of 2K in 0ms Result set of 3K in 1ms Result set of 5K in 1ms Result set of 8K in 2ms Total time 6ms
Result set of 13K in 5ms Result set of 21K in 7ms Result set of 34K in 12ms Result set of 55K in 15ms Total time 40ms
Result set of 100K in 58ms Result set of 200K in 133ms Result set of 300K in 151ms Result set of 500K in 200ms Total time 544ms
Result set of 1M in 783ms Result set of 2M in 1079ms Result set of 3M in 1372ms Result set of 5M in 2601ms Total time 5837ms
Result set of 8M in 5381ms Result set of 13M in 7838ms Total time 13219ms


Testing Implementation thoas
Result set of 1K in 0ms Result set of 2K in 0ms Result set of 3K in 0ms Result set of 5K in 0ms Result set of 8K in 1ms Total time 4ms
Result set of 13K in 5ms Result set of 21K in 7ms Result set of 34K in 9ms Result set of 55K in 11ms Total time 33ms
Result set of 100K in 44ms Result set of 200K in 75ms Result set of 300K in 92ms Result set of 500K in 126ms Total time 339ms
Result set of 1M in 466ms Result set of 2M in 962ms Result set of 3M in 963ms Result set of 5M in 1237ms Total time 3629ms
Result set of 8M in 5073ms Result set of 13M in 5978ms Total time 11051ms


Testing Implementation otp
Result set of 1K in 0ms Result set of 2K in 0ms Result set of 3K in 0ms Result set of 5K in 1ms Result set of 8K in 1ms Total time 3ms
Result set of 13K in 1ms Result set of 21K in 3ms Result set of 34K in 7ms Result set of 55K in 8ms Total time 20ms
Result set of 100K in 14ms Result set of 200K in 29ms Result set of 300K in 43ms Result set of 500K in 65ms Total time 153ms
Result set of 1M in 390ms Result set of 2M in 579ms Result set of 3M in 454ms Result set of 5M in 1180ms Total time 2604ms
Result set of 8M in 4166ms Result set of 13M in 5133ms Total time 9299ms

* Encoding use JSON library for OTP PR

Uses the PR'd Json library to OTP for encoding, and passing in a bespoke override on the encode function to avoid the double-pass to convert into a map.

This appears to be notably faster than mochijson2 and thoas.

* Tidy-up
@martinsumner
Copy link
Author

martinsumner commented Feb 21, 2024

Relative performance at different result sizes - mochijson2 vs OTP Json - OTP 22/24/26:

JSON Compare - Relative

Focus on proposed JSON library for OTP.

The riak_kv_clusteraae_fsm has also been reverted back to mochijson2.  The format of leveled_tictac:export_tree isn't compatible with the OTP json library - rather than over-complicate the PR, as performance is not critical for these queries, keep the old implementation.
@martinsumner martinsumner marked this pull request as draft February 22, 2024 13:30
Without compatibility changes
Compatibility changes required for OTP 22 - 26
src/riak_kv_wm_json.erl Outdated Show resolved Hide resolved
* Update and include quickcheck properties

* Output too verbose with a collect.
@martinsumner martinsumner marked this pull request as ready for review March 18, 2024 11:30
@martinsumner martinsumner merged commit aec41e6 into nhse-develop Mar 28, 2024
3 checks passed
@martinsumner martinsumner deleted the nhse-d32-nhskv.i17 branch March 28, 2024 15:33
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