From 1f808645b0e4b65042b2ebf437257d0f7387d80e Mon Sep 17 00:00:00 2001 From: Matt Lord Date: Wed, 28 Feb 2024 10:41:04 -0500 Subject: [PATCH] Minor changes from self review Signed-off-by: Matt Lord --- go/textutil/strings.go | 9 ++++++--- go/textutil/strings_test.go | 1 + go/vt/vttablet/tabletmanager/vreplication/utils.go | 7 +++++-- go/vt/vttablet/tabletmanager/vreplication/utils_test.go | 2 +- go/vt/vttablet/tabletmanager/vreplication/vreplicator.go | 2 +- 5 files changed, 14 insertions(+), 7 deletions(-) diff --git a/go/textutil/strings.go b/go/textutil/strings.go index 0b7f0b66095..33b5bd5dc7e 100644 --- a/go/textutil/strings.go +++ b/go/textutil/strings.go @@ -142,14 +142,17 @@ func Title(s string) string { s) } +// TruncateText truncates the provided text to the specified length using the +// provided indicator in place of the truncated text in the specified location +// of the original. func TruncateText(text string, maxLen int, location TruncationLocation, truncationIndicator string) (string, error) { - if len(truncationIndicator)+2 >= maxLen { - return "", fmt.Errorf("the truncation indicator is too long for the provided text") - } ol := len(text) if ol <= maxLen { return text, nil } + if len(truncationIndicator)+2 >= maxLen { + return "", fmt.Errorf("the truncation indicator is too long for the provided text") + } switch location { case TruncationLocationMiddle: prefix := int((float64(maxLen) * 0.5) - float64(len(truncationIndicator))) diff --git a/go/textutil/strings_test.go b/go/textutil/strings_test.go index ce20cf19f1c..ea940605fd1 100644 --- a/go/textutil/strings_test.go +++ b/go/textutil/strings_test.go @@ -280,6 +280,7 @@ func TestTruncateText(t *testing.T) { } else { require.NoError(t, err) require.Equal(t, tt.want, val) + require.LessOrEqual(t, len(val), tt.maxLen) } }) } diff --git a/go/vt/vttablet/tabletmanager/vreplication/utils.go b/go/vt/vttablet/tabletmanager/vreplication/utils.go index cd12e4cc9e5..6ea647608a2 100644 --- a/go/vt/vttablet/tabletmanager/vreplication/utils.go +++ b/go/vt/vttablet/tabletmanager/vreplication/utils.go @@ -34,6 +34,9 @@ import ( const ( vreplicationLogTableName = "vreplication_log" + // Truncate values in the middle to preserve the end of the message which typically contains the + // error text. + truncationLocation = textutil.TruncationLocationMiddle // This comes from the fact that the message column in the vreplication_log table is of type TEXT. maxVReplicationLogMessageLen = 65535 // This comes from the fact that the message column in the vreplication table is varbinary(1000). @@ -41,7 +44,7 @@ const ( ) // vrepliationLogTruncationStr is the string that is used to indicate that a message has been -// truncated, in the middle, before being inserted into the vreplication_log table. +// truncated before being inserted into one of the vreplication sidecar tables. var vreplicationLogTruncationStr = fmt.Sprintf(" ... %s ... ", sqlparser.TruncationText) const ( @@ -111,7 +114,7 @@ func insertLog(dbClient *vdbClient, typ string, vreplID int32, state, message st // We perform the truncation, if needed, in the middle of the message as the end of the message is likely to // be the most important part as it often explains WHY we e.g. failed to execute an INSERT in the workflow. if len(message) > maxVReplicationLogMessageLen { - message, err = textutil.TruncateText(message, maxVReplicationLogMessageLen, textutil.TruncationLocationMiddle, vreplicationLogTruncationStr) + message, err = textutil.TruncateText(message, maxVReplicationLogMessageLen, truncationLocation, vreplicationLogTruncationStr) if err != nil { log.Errorf("Could not insert vreplication_log record because we failed to truncate the message: %v", err) return diff --git a/go/vt/vttablet/tabletmanager/vreplication/utils_test.go b/go/vt/vttablet/tabletmanager/vreplication/utils_test.go index 5eb9d7ddd34..dd0bec4c6c1 100644 --- a/go/vt/vttablet/tabletmanager/vreplication/utils_test.go +++ b/go/vt/vttablet/tabletmanager/vreplication/utils_test.go @@ -81,7 +81,7 @@ func TestInsertLogTruncation(t *testing.T) { err error ) if tc.expectTruncation { - messageOut, err = textutil.TruncateText(tc.message, maxVReplicationLogMessageLen, textutil.TruncationLocationMiddle, vreplicationLogTruncationStr) + messageOut, err = textutil.TruncateText(tc.message, maxVReplicationLogMessageLen, truncationLocation, vreplicationLogTruncationStr) require.NoError(t, err) require.True(t, strings.HasPrefix(messageOut, tc.message[:1024])) // Confirm we still have the same beginning require.True(t, strings.HasSuffix(messageOut, tc.message[len(tc.message)-1024:])) // Confirm we still have the same end diff --git a/go/vt/vttablet/tabletmanager/vreplication/vreplicator.go b/go/vt/vttablet/tabletmanager/vreplication/vreplicator.go index 224a1141d77..deb23aeb3f9 100644 --- a/go/vt/vttablet/tabletmanager/vreplication/vreplicator.go +++ b/go/vt/vttablet/tabletmanager/vreplication/vreplicator.go @@ -452,7 +452,7 @@ func (vr *vreplicator) readSettings(ctx context.Context, dbClient *vdbClient) (s } func (vr *vreplicator) setMessage(message string) (err error) { - message, err = textutil.TruncateText(message, maxVReplicationMessageLen, textutil.TruncationLocationMiddle, vreplicationLogTruncationStr) + message, err = textutil.TruncateText(message, maxVReplicationMessageLen, truncationLocation, vreplicationLogTruncationStr) if err != nil { return err }