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

Mas o34 upstream.d34 #8

Merged
merged 5 commits into from
Dec 3, 2024
Merged

Mas o34 upstream.d34 #8

merged 5 commits into from
Dec 3, 2024

Conversation

martinsumner
Copy link

Bring openriak-3.4 up-to-date with develop-3.4

martinsumner and others added 5 commits November 13, 2024 13:37
* Add eqwalizer and clear for codec & sst

The eqwalizer errors highlighted the need in several places for type clarification.

Within tests there are some issue where a type is assumed, and so ignore has been used to handle this rather than write more complex code to be explicit about the assumption.

The handling of arrays isn't great by eqwalizer - to be specific about the content of array causes issues when initialising an array.  Perhaps a type (map maybe) where one can be more explicit about types might be a better option (even if there is a minimal performance impact).

The use of a ?TOMB_COUNT defined option complicated the code much more with eqwalizer.  So for now, there is no developer option to disable ?TOMB_COUNT.

Test fixes required where strings have been used for buckets/keys not binaries.

The leveled_sst statem needs a different state record for starting when compared to other modes.  The state record has been divided up to reflect this, to make type management easier.  The impact on performance needs to be tested.

* Update ct tests to support binary keys/buckets only

* Eqwalizer for leveled_cdb and leveled_tictac

As array is used in leveled_tictac - there is the same issue as with leveled_sst

* Remove redundant indirection of leveled_rand

A legacy of pre-20 OTP

* Morde modules eqwalized

ebloom/log/util/monitor

* Eqwalize further modules

elp eqwalize leveled_codec; elp eqwalize leveled_sst; elp eqwalize leveled_cdb; elp eqwalize leveled_tictac; elp eqwalize leveled_log; elp eqwalize leveled_monitor; elp eqwalize leveled_head; elp eqwalize leveled_ebloom; elp eqwalize leveled_iclerk

All concurrently OK

* Refactor unit tests to use binary() no string() in key

Previously string() was allowed just to avoid having to change all these tests.  Go through the pain now, as part of eqwalizing.

* Add fixes for penciller, inker

Add a new ?IS_DEF macro to replace =/= undefined.

Now more explicit about primary, object and query keys

* Further fixes

Need to clarify functions used by runner - where keys , query keys and object keys are used

* Further eqwalisation

* Eqwalize leveled_pmanifest

Also make implementation independent of choice of dict - i.e. one can save a manifest using dict for blooms/pending_deletions and then open a manifest with code that uses a different type.  Allow for slow dict to be replaced with map.

Would not be backwards compatible though, without further thought - i.e. if you upgrade then downgrade.

Redundant code created by leveled_sst refactoring removed.

* Fix backwards compatibility issues

* Manifest Entry to belong to leveled_pmanifest

There are two manifests - leveled_pmanifest and leveled_imanifest.  Both have manifest_entry() type objects, but these types are different.  To avoid confusion don't include the pmanifest manifest_entry() within the global include file - be specific that it belongs to the leveled_pmanifest module

* Ignore elp file - large binary

* Update src/leveled_pmem.erl

Remove unnecessary empty list from type definition

Co-authored-by: Thomas Arts <[email protected]>

---------

Co-authored-by: Thomas Arts <[email protected]>
* Make log functions exportable

To make it easier to switch to logger in kv_index_tictactree - export the log functions from leveled so that they can be reused

* Changes post review

Added brief description of the module to explain why the approach to logging is used.

* Result of log should be `ok`
* Support sub-key queries

Also requires a refactoring of types.

In head-only mode - the metadata in the ledger is just the value, and the value can be anything.  So metadata() definition needs to reflect that.

There are then issues with appdefined functions for extracting metadata.  In theory an appdefined function could extract some unsopprted type.  So made explicit that the appdefined function must extract std_metadata() as metadata - otherwise functionality will not work.

This means that if it is an object key, that is not a ?HEAD key, then the Metadata must be a tuple (of either Riak or Standard type).

* Fix coverage issues
* Add test to replicate issue 459

Nothing actually crashes due to the issue - but looking at the logs there is the polarised stats associated with the issue.  When  merging into L3, you would normally expect to merge into 4 files - but actually we see FileCounter occasionally spiking.

* Add partial merge support

There is a `max_mergebelow` size which can be a positive integer, or infinity.  It defaults to 32.

If a merge from Level N covers less than `max_mergebelow` files in level N + 1 - the merge will proceesd as before.  If it has >= `max_mergebelow`, the merge will be curtailed when `max_mergebelow div 2` files have been created at that level.  The remainder for Level N will then be written, as well as for Level N + 1 up to the next whole file that has no yet been touched by the merge.

The backlog that prompted the merge will still exist - as the files in Level N have not been changed.  However, it is likely the next file picked will not be the same one, and will in probability have a lower number of files to merge (as the average is =< 8).

This will stop progress from being halted by long merge jobs, as they will exit out in a safe way after partial completion.  In the case where the majority of files covered  do not require a merge, then those files will be skipped the next time the remainder file is picked up for merge at Level N
@martinsumner martinsumner merged commit 539c2ab into openriak-3.4 Dec 3, 2024
2 checks passed
@martinsumner martinsumner deleted the mas-o34-upstream.d34 branch December 3, 2024 10:27
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.

1 participant