DiceDB
#421: Config file support
This PR introduce more structured way of using config in DiceDB
1 introducing installation script:
- creates appropriate user grp, files and folder to support configuration.
- reduce user complexity of manual setup
[!NOTE] requires default config file and build file of Dice in cloud to copy
2 config file support using viper:
-
our db uses
.tomlfile for config and can support live reloading of config thanks to viper -
config file demo:
[Server]
Addr = "0.0.0.0"
Port = 7379
KeepAlive = 60
Timeout = 300
MaxConn = 0
ShardCronFrequency = "1s"
MultiplexerPollTimeout = "100ms"
MaxClients = 20000
MaxMemory = 0
EnvictionPolicy = "allkeys-lru"
PersistanceEnabled = true
[Auth]
UserName = "dice"
Password = ""
[Network]
IOBufferLength = 512
IOBufferLengthMAX = 51200
@vinitparekh17 thanks for getting this very important PR out @lucifercr07 thanks for the thorough review, lets make sure we have this important change in asap. Today the configs are global variables which is dangerous, this PR will make the config management much saner.
Config file test cases implemented and locally tested, PersistenceEnabled flag won't cause any test to fail (except flakiness issues due to io timeout, the present cause)
@soumya-codes @lucifercr07 I think we are good to go now.
Please rebase this PR.
@JyotinderSingh Cleaned up the mess of conflicts, rebase completed
If we do not include the shell script, will these changes still work out of the box? We shouldn't require the user to set up user groups and permissions just to run the database (even running a script shouldn't be necessary).
If we do not include the shell script, will these changes still work out of the box? We shouldn't require the user to set up user groups and permissions just to run the database (even running a script shouldn't be necessary).
It will work seamlessly, but the catch is that the database won’t be able to access /etc/<dbname>/<fileName>, unlike Redis.
current behaviour will be as follow as:
- Dice will use the default configuration if it starts in the usual way.
- Dice will read the config if started with the
-cflag (file path of the config file). - Dice will create a configuration file if started with the
-oflag (destination path to create the config), provided it has sufficient permissions to access that path.
If we do not include the shell script, will these changes still work out of the box? We shouldn't require the user to set up user groups and permissions just to run the database (even running a script shouldn't be necessary).
It will work seamlessly, but the catch is that the database won’t be able to access
/etc/<dbname>/<fileName>, unlike Redis.current behaviour will be as follow as:
Dice will use the default configuration if it starts in the usual way.
Dice will read the config if started with the
-cflag (file path of the config file).Dice will create a configuration file if started with the
-oflag (destination path to create the config), provided it has sufficient permissions to access that path.
Got it. Please document this behavior and information in the README.
We can add the shell script as a separate change (just mention what permissions the user needs to set for the directory stuff to work so that they can do it manually and don't have to depend on a script)
If we do not include the shell script, will these changes still work out of the box? We shouldn't require the user to set up user groups and permissions just to run the database (even running a script shouldn't be necessary).
It will work seamlessly, but the catch is that the database won’t be able to access
/etc/<dbname>/<fileName>, unlike Redis.current behaviour will be as follow as:
- Dice will use the default configuration if it starts in the usual way.
- Dice will read the config if started with the
-cflag (file path of the config file).- Dice will create a configuration file if started with the
-oflag (destination path to create the config), provided it has sufficient permissions to access that path.Got it. Please document this behavior and information in the README.
We can add the shell script as a separate change (just mention what permissions the user needs to set for the directory stuff to work so that they can do it manually and don't have to depend on a script)
Changes made
Had a few minor comments, please resolve them. Will merge once done.
Had a few minor comments, please resolve them. Will merge once done.
We are good to go 👍
@vinitparekh17 there seem to be some data races happening. Can you look into it?
@vinitparekh17 there seem to be some data races happening. Can you look into it?
Checking
@JyotinderSingh Isolated config test in order to prevent race condition.
@JyotinderSingh Isolated config test in order to prevent race condition.
Why did we remove test case for scenario 5?
@JyotinderSingh Isolated config test in order to prevent race condition.
Why did we remove test case for scenario 5?
Scenario 4 and 5 is almost identical so thought it was unnecessary
Anyways have to look into it coz in my machine it works in Github action it fails
@JyotinderSingh It will work now, hopefully
added back scenario 5, made necessary changes to make it cleaner and accurate and now tested it on cloud instance.
btw, I am really sorry for your inconvenience