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

test(runtime-config): Add some tests to make sure data is cached #6586

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

john-z-yang
Copy link
Member

@john-z-yang john-z-yang commented Nov 20, 2024

Overview

We ran into an incident where we accidentally did not store runtime config into our cache and retrieved it from redis every time the get_runtime_config is called. This pr tries to add a couple of tests to assert the caching behaviour.

@john-z-yang john-z-yang requested a review from a team as a code owner November 20, 2024 22:44
@john-z-yang john-z-yang force-pushed the test-runtime-config-cache branch 2 times, most recently from c979a7d to aa7dbf5 Compare November 20, 2024 22:45
@volokluev
Copy link
Member

Good start but what we really want to check is that get_str_config actually stores a value in cache if it retrieves it

@john-z-yang
Copy link
Member Author

john-z-yang commented Nov 20, 2024

@volokluev I addedtest_runtime_config_is_stored_in_cache. Let me know how this looks

Comment on lines +80 to +93
#[test]
fn test_runtime_config_invalidates_cache_outside_deadline() {
{
CONFIG.write().insert(
"key3".to_string(),
(
Some("value3".to_string()),
Deadline::new(Duration::from_secs(0)),
),
);
}
let config = get_str_config("key3");
assert_eq!(config.unwrap(), None);
CONFIG.write().clear();
Copy link
Member Author

@john-z-yang john-z-yang Nov 20, 2024

Choose a reason for hiding this comment

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

I wanted to set this deadline to maybe 1 sec from now, and sleep for 1 sec to retrieve it, like so

#[test]
    fn test_runtime_config_invalidates_cache_outside_deadline() {
        {
            CONFIG.write().insert(
                "key3".to_string(),
                (
                    Some("value3".to_string()),
                    Deadline::new(Duration::from_secs(1)), // 1 instead of 0
                ),
            );
        }
        thread::sleep(Duration::from_secs(900));
        let config = get_str_config("key3");
        assert_eq!(config.unwrap(), None);
        CONFIG.write().clear();
    }

But for some reason this fails, and the debugger shows that coarsetime::Instant::recent() does not appear to be advancing

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