[cli] estimate-gc with custom TTL

[neeral]

Release note:
cockroach debug estimate-gc now allows user to specify TTL period,
with a default of 24 hours.

[cockroach-teamcity]

This change is 

[cockroach-teamcity]

This change is 

[neeral]

See cockroachlabs/rubrik#25

[neeral]

See cockroachlabs/rubrik#25

[nvanbenschoten]

Thanks @neeral!

Reviewable status: complete! 0 of 0 LGTMs obtained

---

pkg/cli/debug.go, line 460 at r1 (raw file):

ranges individually.

Uses a hard-coded GC policy, with a default 24 hour TTL, for old versions.

This is no longer true, right? It now only uses the default TTL is one isn't provided.

---

pkg/cli/debug.go, line 471 at r1 (raw file):

	var rangeID roachpb.RangeID
	var gcTtlInSeconds int32 = 24 * 60 * 60 /* 1 day */

nit: (24 * time.Hour).Seconds() reads a little better.

---

pkg/cli/debug.go, line 476 at r1 (raw file):

		var err error
		if rangeID, err = parseRangeID(args[1]); err != nil {
			return err

Wrap these errors to give them some context.

[neeral]

Reviewable status: complete! 0 of 0 LGTMs obtained

---

pkg/cli/debug.go, line 460 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

This is no longer true, right? It now only uses the default TTL is one isn't provided.

A GC policy is defined by zones. We still have a hard-coded policy of a single zone. What's changed is the TTL for that zone.

---

pkg/cli/debug.go, line 476 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Wrap these errors to give them some context.

Done.

[nvanbenschoten]

Reviewable status: complete! 0 of 0 LGTMs obtained

---

pkg/cli/debug.go, line 460 at r1 (raw file):

Previously, neeral (Neeral Dodhia) wrote…

A GC policy is defined by zones. We still have a hard-coded policy of a single zone. What's changed is the TTL for that zone.

My issue is with the term "hard-coded". I'd say "configurable".

---

pkg/cli/debug.go, line 479 at r2 (raw file):

		}
		var gcTTLInSecondRangeID roachpb.RangeID
		if gcTTLInSecondRangeID, err = parseRangeID(args[2]); err != nil {

Why are we using parseRangeID for this? Let's just use strconv.ParseInt directly.

[neeral]

Reviewable status: complete! 0 of 0 LGTMs obtained

---

pkg/cli/debug.go, line 460 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

My issue is with the term "hard-coded". I'd say "configurable".

Fixed.

---

pkg/cli/debug.go, line 479 at r2 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Why are we using parseRangeID for this? Let's just use strconv.ParseInt directly.

I wanted to keep the logic for checking errors from strconv.ParseInt and whether int is positive. I've put this into a helper function.

[nvanbenschoten]

thanks @neeral!

bors r+

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale)

[craig]

👎 Rejected by code reviews

[craig]

👎 Rejected by code reviews

[nvanbenschoten]

bors r+

[nvanbenschoten]

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)

[craig]

Build succeeded

• GitHub CI (Cockroach)