Contribute Threadsave

[nitaicaro]

No description provided.

[nitaicaro]

@JimB123 - I took this from EC but am wondering whether we should rethink this. If threadsave is running, server.child_type is NONE, which I feel should imply server.rdb_child_type should also be NONE. But it is actually set to DISK/SOCKET in this case.

[nitaicaro]

The current state is a little confusing as we have multiple checks around the code such as:

• isSaveInProgress() - checks server.rdb_child_type - disk or socket
• checking server.rdb_child_type directly
• hasActiveChildProcess() - works on all types of rdb child (such as RDB, AOF, ...)
• server.child_type == CHILD_TYPE_RDB - is specifically a fork bgsave running?
• server.cur_bgsave_type

So I'm thinking whether this can be a good opportunity to clean this up.
I'm thinking no direct checks on server vars - only function calls such as

isThreadBgsaveInProgress(){
...
}
isForkBgsaveInProgress(){
...
}
isBgsaveInProgress(){
 return isThreadBgsaveInProgress() || isForkBgsaveInProgress()
}
isDiskBasedSaveInProgress(){
return server.rdb_child_type == TYPE_DISK
}
hasActiveChildProcess(){
   return server.child_pid != -1
}
...
...

also rename rdb_child_type to something like rdb_save_target

[JimB123]

I agree that some cleanup may be warranted. The code you're looking at is very old.
Amzsave (before my time) may have overloaded some of the OSS variables. Threadsave just tried to maintain compatibility with Amzsave.

A concern I'd have about renaming things is that if these show in INFO - it's considered a breaking change. And if you rename the variable internally, but show it in INFO with the old (compatible) change, it may be confusing.

I think you have a lot of latitude in changing threadsave - but less latitude in changing the OSS INFO fields.

[nitaicaro]

Done. In the last few commits

[nitaicaro]

.