Feature: Add `setOrGet` Function to Cache Implementation

ndjuric-bit commented 10 months ago

Overview:

This pull request introduces a new feature to our cache implementation by adding the SetOrGet function. This function allows clients to efficiently set a cache entry if it doesn't exist or retrieve the existing value associated with a given key. It returns the actual value, a boolean indicating whether the entry was loaded from the cache or not, and any potential error.

Function Signature::

SetOrGet(key string, entry []byte) (actual []byte, loaded bool, err error)

Parameters:

key: A string representing the cache key to look up.
entry: A byte slice containing the value to set in the cache if the key doesn't exist.

Return Values:

actual: The actual value associated with the key in the cache, which may be the newly set value or the existing cached value.
loaded: A boolean indicating whether the entry was loaded from the cache (true) or if it was set with the provided entry (false).
err: An error that is returned in case of any issues during the set or get operation.

Example:

actualValue, loadedFromCache, err := cache.SetOrGet("my_key", []byte("new_value"))
if err != nil {
    // Handle the error.
} else {
    if loadedFromCache {
        // The value was loaded from the cache.
        fmt.Printf("Value for key 'my_key' found in the cache: %s\n", actualValue)
    } else {
        // The value was set in the cache.
        fmt.Printf("Value for key 'my_key' was set to: %s\n", actualValue)
    }
}

Testing:

This pull request includes unit tests to ensure the correct behavior of the SetOrGet function. The tests cover various scenarios, including cache hits and cache misses.

Impact on Existing Code:

This change introduces a new function to cache system. Existing code that uses the cache may benefit from this function for efficient get-or-set operations, but it should not negatively impact the existing functionality. This change is designed to be backward compatible.

Documentation:

The function's usage has been documented, and code comments have been updated to reflect the introduction of the SetOrGet function.

Reviewer Instructions:

Please review the code changes and documentation to ensure that the SetOrGet function is correctly implemented and well-documented. Verify that the function behaves as expected, handles errors gracefully, and is a useful addition to our cache system. In case of hash collision behaviour of get function is held plus old data is deleted and new one is stored

Testing Instructions: Verify that the unit tests pass and consider adding additional test cases as needed to cover any edge cases.

Changelog:

Added SetOrGet function to the cache module.
Updated documentation and comments to include usage examples.

codecov-commenter commented 10 months ago

Codecov Report

Merging #383 (dd4212d) into main (58fe2d2) will increase coverage by 0.27%. The diff coverage is 82.75%.

:exclamation: Current head dd4212d differs from pull request most recent head 4c79098. Consider uploading reports for the commit 4c79098 to get more accurate results

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##             main     #383      +/-   ##
==========================================
+ Coverage   88.66%   88.94%   +0.27%     
==========================================
  Files          15       15              
  Lines         794      823      +29     
==========================================
+ Hits          704      732      +28     
  Misses         76       76              
- Partials       14       15       +1

Files	Coverage Δ
bigcache.go	`93.18% <100.00%> (+0.21%)`	:arrow_up:
shard.go	`90.97% <80.00%> (+0.47%)`	:arrow_up:

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 58fe2d2...4c79098. Read the comment docs.

janisz commented 10 months ago

@ndjuric-bit Thanks for this. I think we need to solve issue with concurrent updates during setOrGet with current implementation it's not locked so there might an override. I think this would be a rare case but possible so it needs to be documented. I also miss some tests how it behaves when error happens.

ndjuric-bit commented 10 months ago

@janisz Is there something needed from my side more ?

allegro / bigcache