From 5c146ca57ad5bc8e91d8af9ea59bd5b0f49d6361 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 3 Mar 2025 14:42:43 +0100 Subject: [PATCH] Synchronize logging in cmdio (#2418) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Changes Add mutex synchronization in cmdio logger Log() method. ## Why Since we issue multiple calls to underlying writer, we should lock the whole method, otherwise we can get broken messages. One that can be easily reproduced today is ``` hyperfine -m 100 --show-output 'go test ./acceptance -run ^TestAccept$/^bundle$/^artifacts$/^whl_multiple$ -count=1' ... -Uploading my_test_code-0.0.1-py3-none-any.whl... -Uploading my_test_code_2-0.0.1-py3-none-any.whl... +Uploading my_test_code-0.0.1-py3-none-any.whl...Uploading my_test_code_2-0.0.1-py3-none-any.whl... Error: Command terminated with non-zero exit code 1 in benchmark iteration 54. Use the '-i'/'--ignore-failure' option if you want to ignore this. Alternatively, use the '--show-output' option to debug what went wrong. ``` An alternative could be to prepare a message fully in a local buffer and write it in one call (I’m assuming underlying writer is still synchronized). However, that’s more complicated and unclear if it’s worth it, perf-wise. ## Tests With this change I’m running the same hyperfine command with 1000 iterations with no failures. --- libs/cmdio/logger.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libs/cmdio/logger.go b/libs/cmdio/logger.go index 48b76ce42..7d369a8a0 100644 --- a/libs/cmdio/logger.go +++ b/libs/cmdio/logger.go @@ -9,6 +9,7 @@ import ( "io" "os" "strings" + "sync" "github.com/databricks/cli/libs/flags" "github.com/manifoldco/promptui" @@ -29,6 +30,8 @@ type Logger struct { // If true, indicates no events have been printed by the logger yet. Used // by inplace logging for formatting isFirstEvent bool + + mutex sync.Mutex } func NewLogger(mode flags.ProgressLogFormat) *Logger { @@ -216,6 +219,8 @@ func (l *Logger) writeInplace(event Event) { } func (l *Logger) Log(event Event) { + l.mutex.Lock() + defer l.mutex.Unlock() switch l.Mode { case flags.ModeInplace: if event.IsInplaceSupported() {