bucket: "inspect --sort-by" is not numeric-aware

[belm0]

Thanos, Prometheus and Golang version used: v0.11.0

What happened: bucket inspect --sort-by UNTIL-DOWN does not produce expected ordering

What you expected to happen: sort numerically by time duration

If it's too complex to support alpha vs. numeric, sorting direction, etc. perhaps it would be better to remove the --sort-by feature and leave it to post-processing...

How to reproduce it (as minimally and precisely as possible):

$ /bin/thanos bucket inspect --sort-by UNTIL-DOWN
|            ULID            |        FROM         |        UNTIL        |  RANGE   | UNTIL-DOWN |  #SERIES  |    #SAMPLES    |  #CHUNKS   | COMP-LEVEL | COMP-FAILED |    LABELS    | RESOLUTION |  SOURCE   |
|----------------------------|---------------------|---------------------|----------|------------|-----------|----------------|------------|------------|-------------|--------------|------------|-----------|
...
| 01E5096G1P88MBXNATDFDPNNRG | 01-04-2020 06:00:00 | 03-04-2020 12:00:00 | 54h0m0s  | -14h0m0s   | 2,316,976 | 3,964,484,856  | 37,540,413 | 4          | false       | replica=main | 0s         | sidecar   |
| 01E5H7MQCQ5Q86095CC1JSJ8AV | 26-12-2019 12:00:00 | 04-01-2020 12:00:00 | 216h0m0s | -176h0m0s  | 842,250   | 1,978,856,089  | 19,461,079 | 6          | false       | replica=main | 0s         | compactor |
| 01E5H6MFM2A9QSMCSX2JK441T8 | 08-04-2020 00:00:00 | 10-04-2020 00:00:00 | 48h0m0s  | -8h0m0s    | 2,207,280 | 3,711,359,114  | 34,641,807 | 3          | false       | replica=main | 0s         | compactor |
| 01E5HCVWQSC1492701AWVKM0CF | 09-03-2020 18:00:00 | 12-03-2020 00:00:00 | 54h0m0s  | 186h0m0s   | 1,880,277 | 580,895,412    | 5,115,790  | 4          | true        | replica=main | 5m0s       | compactor |
| 01E5HHR9V8NGYBC7CTAVYX766E | 27-03-2020 18:00:00 | 30-03-2020 00:00:00 | 54h0m0s  | 186h0m0s   | 1,688,583 | 701,582,637    | 5,788,266  | 4          | true        | replica=main | 5m0s       | compactor |
| 01E5HJTRFBPC2PG44J5BQA170R | 05-03-2020 06:00:00 | 07-03-2020 12:00:00 | 54h0m0s  | 186h0m0s   | 1,795,213 | 572,997,620    | 5,012,678  | 4          | true        | replica=main | 5m0s       | compactor |
| 01E5HQACS5J0RGANE2D9H40W2V | 30-03-2020 00:00:00 | 01-04-2020 06:00:00 | 54h0m0s  | 186h0m0s   | 2,201,667 | 786,975,870    | 6,748,038  | 4          | true        | replica=main | 5m0s       | compactor |
| 01E5HS781NM79E3CT9PBN7GKPR | 01-04-2020 06:00:00 | 03-04-2020 12:00:00 | 54h0m0s  | 186h0m0s   | 2,316,976 | 805,786,196    | 6,953,904  | 4          | false       | replica=main | 5m0s       | compactor |
| 01E5J1HHD53XG3TC3V6NWAEJ8W | 25-03-2020 12:00:00 | 27-03-2020 18:00:00 | 54h0m0s  | 186h0m0s   | 2,031,452 | 716,804,692    | 6,159,897  | 4          | true        | replica=main | 5m0s       | compactor |
| 01E5J4SN7PHQW6Z2WQ3DAZC3Y7 | 07-03-2020 12:00:00 | 09-03-2020 18:00:00 | 54h0m0s  | 186h0m0s   | 1,696,303 | 562,270,851    | 4,858,224  | 4          | true        | replica=main | 5m0s       | compactor |
| 01E5HDVZSCJNRGWJ301YTG4FKT | 08-04-2020 00:00:00 | 10-04-2020 00:00:00 | 48h0m0s  | 192h0m0s   | 2,207,280 | 755,607,249    | 6,784,993  | 3          | false       | replica=main | 5m0s       | compactor |
| 01E526VK93N0JRPNZPR5Y4TNC7 | 03-04-2020 12:00:00 | 04-04-2020 06:00:00 | 18h0m0s  | 22h0m0s    | 1,670,889 | 1,309,978,265  | 11,885,519 | 3          | false       | replica=main | 0s         | sidecar   |
| 01E544MJZP1HD9NCB2YS6SWRKJ | 04-04-2020 06:00:00 | 05-04-2020 00:00:00 | 18h0m0s  | 22h0m0s    | 1,571,817 | 1,250,676,619  | 11,341,306 | 3          | false       | replica=main | 0s         | sidecar   |
...

[belm0]

The human-readable durations will make sort by post-processing tedious.

If we really want to resort to post-process, then an option to output durations as a number (e.g. seconds) would be needed.

Or, a compromise proposal: keep human-readable durations, but used a fixed format suitable for the existing alphabetic sort. And instead of negative durations, output a zero duration to indicate that no event is pending. For example:

UNTIL-DOWN
000h00m00s
000h00m00s
000h00m00s
004h00m00s
022h00m00s
034h00m00s
078h00m00s
186h00m00s

(000h00m00s means no pending event)

Using a fixed format will help readability anyway...

[GiedriusS]

I think it makes sense that we would sort these values by their numerical value, not by the order of alphanumeric characters. It seems like the code at the end of cmd/thanos/bucket.go needs to be improved. Would that work for you?

[belm0]

I see, the code is already doing a numeric sort for FROM / UNTIL since the dates are in DD-MM-YYYY format. (Alphanumeric sort would be sufficient if the format was "YYYY-MM-DD".)

To fully support --sort-by for all columns, and assuming the implementation continues to sort the string representation, these field types need to be supported:

type	example fields	example values	notes
datetime	UNTIL	"03-04-2020 12:00:00"	already implemented
human-readable duration	RANGE, UNTIL-DOWN, RESOLUTION	"54h0m0s", "-14h0m0s"
comma-delimited integer	#SAMPLES, COMP-LEVEL	"3,964,484,856"

It may be better to refactor the code to sort the underlying values, rather than the string representations.

[kadern0]

@belm0 want to have quick a look at my PR? #2416