danielealbano / cachegrand

cachegrand - a modern data ingestion, processing and serving platform built for today's hardware
BSD 3-Clause "New" or "Revised" License
975 stars 34 forks source link

Fix new hashtable upsize concurrency issue #265

Closed danielealbano closed 1 year ago

danielealbano commented 1 year ago

After the initial implementation and merge of the new hashtable some issues were found, after further investigation some bugs were discovered in the upsize implementation and that it was also clashing with the normal operation carried out during the get, set and delete operation.

This PR fixes the bug in the upsize implementation to ensure that everything is always copied and that if there is a bucket marked as temporary, because a write is in progress, it waits for it to be finalized or reverted (although this approach will be changed later to perform better when transactions will be implemented). The PR also fixes the clashing behaviour with the get, set and delete operations, especially with the last 2, as they always to retry when the bucket found changes underneath (as it can be another operation or the migration process).

The PR also includes a new fuzzy test created to ensure that this behaviour is further tested and that everything works as expected.

codecov[bot] commented 1 year ago

Codecov Report

Base: 82.67% // Head: 82.88% // Increases project coverage by +0.21% :tada:

Coverage data is based on head (d2b5110) compared to base (7f32454). Patch coverage: 86.67% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #265 +/- ## ========================================== + Coverage 82.67% 82.88% +0.21% ========================================== Files 158 158 Lines 10240 10257 +17 ========================================== + Hits 8465 8501 +36 + Misses 1775 1756 -19 ``` | Flag | Coverage Δ | | |---|---|---| | unittests | `82.88% <86.67%> (+0.21%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniele+Salvatore+Albano#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/danielealbano/cachegrand/pull/265?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniele+Salvatore+Albano) | Coverage Δ | | |---|---|---| | [...rc/data\_structures/hashtable\_mpmc/hashtable\_mpmc.c](https://codecov.io/gh/danielealbano/cachegrand/pull/265/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniele+Salvatore+Albano#diff-c3JjL2RhdGFfc3RydWN0dXJlcy9oYXNodGFibGVfbXBtYy9oYXNodGFibGVfbXBtYy5j) | `94.03% <86.67%> (+4.83%)` | :arrow_up: | | [src/transaction\_spinlock.h](https://codecov.io/gh/danielealbano/cachegrand/pull/265/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniele+Salvatore+Albano#diff-c3JjL3RyYW5zYWN0aW9uX3NwaW5sb2NrLmg=) | `90.00% <0.00%> (-5.00%)` | :arrow_down: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniele+Salvatore+Albano). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniele+Salvatore+Albano)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.