add snapshot while context , default configs updates

[pratikmishra356]

Problem

Record for config's history after every cac_config changes,
we also encountered problem regarding generating snowflake_id, we were not dropping the acquired mutex lock,
and when generating another snowflake_id , it gets stuck in deadlock kind of situation

Solution

Created config_versions table which will store the config's snapshot after every updates which affect the configs.
Fetch contexts and default_configs and insert into config_Versions table after any updates within the same transaction.
we are explicitly dropping acquired lock after snowflake_id generation
Also accepting tags parameters in every update request so that user can tag every versions

Environment variable changes

NA

Pre-deployment activity

Create db changes

Post-deployment activity

NA

API changes

get_config apis we will now provide priority inside context objects
adding optional tags field in every context, default_config and experiments updates
accepting version queryparam in every get_config apis

Possible Issues in the future

NA

[mahatoankitkumar]

I think we can now deprecate this handler.

[pratikmishra356]

why?

[Datron]

Why add Arc? as per rust docs

Shared references in Rust disallow mutation by default, and Arc is no exception: you cannot generally obtain a mutable reference to something inside an Arc. If you need to mutate through an Arc, use Mutex, RwLock, or one of the Atomic types.

If we aren't mutating the snowflakeIdGenerator, just use Arc otherwise there is no need to add it

[Datron]

@pratikmishra356

[pratikmishra356]

@Datron arc is used to share the same mutex instance
you can check the example for mutex implementation
/// use std::sync::{Arc, Mutex}; /// use std::thread; /// /// let mutex = Arc::new(Mutex::new(0)); /// let c_mutex = Arc::clone(&mutex); /// /// thread::spawn(move || { /// let mut lock = c_mutex.try_lock(); /// if let Ok(ref mut mutex) = lock { /// **mutex = 10; /// } else { /// println!("try_lock failed"); /// } /// }).join().expect("thread::spawn failed"); /// assert_eq!(*mutex.lock().unwrap(), 10);

[Datron]

Can we squash all commits into one?

[Datron]

What happens when someone tries to update tags? Is that possible?

[Datron]

Why bump up to i64?

[pratikmishra356]

@Datron expecting version_id in query params, which is snowflake_id and it is i64

[Datron]

Would like to avoid this patch file. Can we make the type in DB in such a way that we can avoid this?

[pratikmishra356]

We can concatenate tags with comma and store it as string , but then the DB filter query will not be optimised
https://diesel.rs/guides/configuring-diesel-cli.html
https://diesel.rs/guides/migration_guide.html

[pratikmishra356]

What happens when someone tries to update tags? Is that possible?

@Datron currently this is not supported , a different api will be needed
also we need to have list versions api with filter on tags support
these changes can be added later on , this pr already has lot of changes

[ShubhranshuSanjeev]

Can we accept tags as headers? These don't really belong here, and the name is also misleading should be snapshot_tags

[pratikmishra356]

headers can be used when its value is consistent across similar requests or let say it is a global level identifier for requests .
although tags is consistent across all write requests , I think we should not add this in header because probably tags can be changed based on the different operations (c, u, d) on different entities (default_configs, contexts)

also we have other entities and request where this header is of no use ( like functions, types, dimensions, get apis ) so for me it probably seems more close to a create request than a global request identifier
will change tags -> version_tags

@knutties @Datron your thoughts ?

[ShubhranshuSanjeev]

tags is a actually a metadata here, and not exactly the part of the request, are we using this to tag the actual resource we are touching in these request, the answer is no, these tags in fact are used to tag the versions created after the actual defined job for the request is completed.

[ShubhranshuSanjeev]

We are repeating everywhere too, all request types are changed, can't we do something to abstract this out.

[ShubhranshuSanjeev]

@pratikmishra356 , do we have to fail a request because we weren't able to create a snapshot.
What I think, the user never intended to make a snapshot, he just wanted to create a default_config/context/....., creating a snapshot should be our separate task, I feel we really shouldn't fail the endpoints when we fail to create a snapshot

@knutties @Datron , what are you thoughts on this?

[pratikmishra356]

since currently we dont have any process/event based tracking mechanisms to do these operations in async , we might lose track of updates on configs thats why we followed this behavior ,
we assume every updates on default_config and contexts is crucial which results in changing the configs , and since now we are relying only on config_version table for serving configs ( i.e no more db fetch from multiple tables ) it is important to have this both operation in sync