backupccl: deflake TestShowBackup #111675
Conversation
|
This change is |
pkg/ccl/backupccl/show_test.go
Outdated
| @@ -124,7 +124,7 @@ ORDER BY object_type, object_name`, full) | |||
| // Truncate decimal places so Go's very rigid parsing will work. | |||
| // TODO(bardin): Consider using a third-party library for this, or some kind | |||
| // of time-freezing on the test cluster. | |||
| truncateBackupTimeRE := regexp.MustCompile(`^(.*\.[0-9]{2})[0-9]*\+00$`) | |||
| truncateBackupTimeRE := regexp.MustCompile(`^(.*\.\d{2})?[0-9]*\+00$`) | |||
There was a problem hiding this comment.
I'm not sure this is correct. This is making the entire match group optional.
The point of this code is to get a date in a format that will be usable in the time.Parse call below. So, even if it did work, we'd still have a problem since our date won't work with the time.Parse call .
What's interesting is that the whole reason this occurs is because it seems our time formatting code doesn't output a fixed width format for fractional seconds. That is very surprising to me.
We can see this truncation in action and see how very unlucky we got by hitting this. now() provides microsecond level percision:
> SELECT ts::string FROM (SELECT now()::timestamptz as ts)
-> ;
ts
---------------------------------
2023-10-04 10:17:29.093215+00
(1 row)
So, I think to hit this we had to be at .3 seconds to the microsecond. Bad luck for us. If we fix the timestamp using AOST we can see the truncation:
> SELECT ts::string FROM (SELECT now()::timestamptz as ts) AS OF SYSTEM TIME '2023-10-04 10:00:30.300000';
ts
----------------------------
2023-10-04 10:00:30.3+00
(1 row)
Perhaps one potential option to fix this is up where we ask for the timestamp originally, using experimental_strftime in SQL to format the date: SELECT experimental_strftime(now()::timestamptz, '%Y-%m-%d %H:%M:%S.%f'). This doesn't appear to truncate:
> SELECT experimental_strftime(ts, '%Y-%m-%d %H:%M:%S.%f') FROM (SELECT now()::timestamptz as ts) AS OF SYSTEM TIME '2023-10-04 10:00:30.30';
experimental_strftime
------------------------------
2023-10-04 10:00:30.300000
(1 row)
Another option is to change this regexp to accept 1 or 2 trailing decimal places with something like (untested) ^(.*\.\d{1,2})[0-9]*\+00$. Then, in the parse function, we can use a format like: "2006-01-02 15:04:05.99". The 9s rather than 0s in the fractional seconds portion tells Go that there might elided trailing 0s.
There was a problem hiding this comment.
Actually, now that I think about it for another second, perhaps we can remove the regexp all together and change the time.Parse format to something like "2006-01-02 15:04:05.999999Z07" (in the case without strftime).
There was a problem hiding this comment.
the go playground likes your suggestion anyways https://go.dev/play/p/9K0WWyS36S_O
msbutler
force-pushed
the
butler-deflake-show-backup
branch
from
64ec09a to
4be08e3
Compare
This patch simplifies how TestShowBackup parses the stringed timestamp: it removes the manual splitting of date and time and parses the stringed timestamp in one call. Fixes: cockroachdb#111015 Release note: none
msbutler
force-pushed
the
butler-deflake-show-backup
branch
from
4be08e3 to
899fb87
Compare
|
TFTR! bors r=stevendanna |
|
Build succeeded: |
This patch simplifies how TestShowBackup parses the stringed timestamp: it
removes the manual splitting of date and time and parses the stringed timestamp
in one call.
Fixes: #111015
Release note: none