Copy situational configuration using a temporary file and mv to improve concurrency #3359
Conversation
oracle-contributor-agreement
bot
added
the
OCA Verified
|
Note: the main function to copy situation configuration while booting is in startServer.sh and is |
| trace "Copying '$1' to '$2'." | ||
| tmp_file=$(mktemp -p $(dirname $2)) | ||
| cp $1 $tmp_file | ||
| mv $tmp_file $2 |
There was a problem hiding this comment.
Minor comment:
Copying to a tmp file and then moving the file is a little subtle. Perhaps add a comment about why we're doing this?
| @@ -35,9 +35,9 @@ if [ ! "${DYNAMIC_CONFIG_OVERRIDE:-notset}" = notset ]; then | |||
| tgt_file=$tgt_dir/$tgt_file # add back in tgt dir path | |||
There was a problem hiding this comment.
Minor comment:
The 'copyIfChanged' looks like it already contains the diff/continue logic on lines 36 & the chmod logic on lines 39,40, and 41. I think you can you go ahead and delete these lines.
On the other hand, copyIfChanged is a little talkative - it traces even when it decides to skip the copy. This might be a little distracting since the liveness probe is frequently invoked.
| @@ -35,9 +35,9 @@ if [ ! "${DYNAMIC_CONFIG_OVERRIDE:-notset}" = notset ]; then | |||
| tgt_file=$tgt_dir/$tgt_file # add back in tgt dir path | |||
| [ -f "$tgt_file" ] && [ -z "$(diff $local_fname $tgt_file 2>&1)" ] && continue # nothing changed | |||
| trace "Copying file '$local_fname' to '$tgt_file'." | |||
| cp $local_fname $tgt_file # TBD ignore any error? | |||
| copyIfChanged $local_fname $tgt_file | |||
There was a problem hiding this comment.
Note that "copyIfChanged" exits with SEVERE if the copy or chmod fails. My assumption is that's the original reason why the function wasn't originally used here. Is this something we want to happen in the livenessprobe? IIRC, its copy was supposed to be more of a 'best effort'.
There was a problem hiding this comment.
I think I'm okay with the liveness probe failing and the server pod restarting when this copy fails. The other choice is that the server continues running with the old configuration for an unknown amount of time.
|
Tests of this functionality pass: https://build.weblogick8s.org:8443/job/weblogic-kubernetes-operator-kind-new/12448/ |
|
Kudos, SonarCloud Quality Gate passed!
|
…ve concurrency (oracle#3359) * Use mv while updating situational configuration files to improve concurrency
…ve concurrency (#3359) * Use mv while updating situational configuration files to improve concurrency
The idea is to copy the source file to a temporary file in the same directory as the target location and then use
mvto update the target location atomically.