ttljob: add row statistics

[otan]

This commit adds the ability for the TTL job to generate statistics on
number of rows and number of expired rows on the table. This is off by
default, controllable by the ttl_automatic_stats_poll_interval storage
parameter syntax.

Release justification: low risk high benefit changes to new
functionality

[cockroach-teamcity]

This change is 

[rafiss]

small suggestions

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @otan and @rafiss)

---

pkg/sql/paramparse/paramparse.go, line 60 at r1 (raw file):

}

// DatumAsDuration transforms a tree.TypedExpr containing a Datum into a float.

nit: into a Duration

---

pkg/sql/paramparse/paramparse.go, line 79 at r1 (raw file):

		d = v.Duration
	default:
		err = pgerror.Newf(pgcode.InvalidParameterValue,

should we allow DInts here? doesn't seem that important, just curious

---

pkg/sql/ttl/ttljob/ttljob.go, line 373 at r1 (raw file):

	)

	statsCloseCh := make(chan struct{})

nit: should the channel have a size of 1?

---

pkg/sql/ttl/ttljob/ttljob.go, line 408 at r1 (raw file):

	if ttlSettings.RowStatsPollInterval != 0 {
		g.GoCtx(func(ctx context.Context) error {

random thought: i'm not familiar with this part of the code, but should this use RunAsyncTask?

---

pkg/sql/ttl/ttljob/ttljob.go, line 566 at r1 (raw file):

					ctx,
					c.opName,
					txn,

nit: seems like it could be simpler to pass in a nil txn. then you can get rid of the DB.Txn and SET TRANSACTION parts, just need to keep:

				datums, err := execCfg.InternalExecutor.QueryRow(
					ctx,
					c.opName,
					nil,
					fmt.Sprintf(c.query, details.TableID),
					c.args...,
				)
				if err != nil {
					return err
				}
				c.gauge.Update(int64(*datums[0].(*tree.DInt)))
				return nil

and modify c.query so it has an AOST clause

---

pkg/sql/ttl/ttljob/ttljob.go, line 573 at r1 (raw file):

					return err
				}
				c.gauge.Update(int64(*datums[0].(*tree.DInt)))

nit: use tree.MustBeDInt

[otan]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss)

---

pkg/sql/paramparse/paramparse.go, line 79 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

should we allow DInts here? doesn't seem that important, just curious

not imo, would that be s or ms or ns ...

---

pkg/sql/ttl/ttljob/ttljob.go, line 373 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

nit: should the channel have a size of 1?

no, it's just closed and not used in any way.

---

pkg/sql/ttl/ttljob/ttljob.go, line 408 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

random thought: i'm not familiar with this part of the code, but should this use RunAsyncTask?

runasynctask doesn't have a way to wait(). i'd rather goroutines be accounted for (for now) and block wait at the end

[rafiss]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @otan and @rafiss)

---

pkg/sql/ttl/ttljob/ttljob.go, line 420 at r2 (raw file):

					return nil
				case <-time.After(ttlSettings.RowStatsPollInterval):
					fetchStatistics(ctx, p.ExecCfg(), details, metrics, aostDuration)

hm, obviously this compiled, but a bit confused how since nothing gets returned in this branch.

[otan]

bors r=rafiss

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss)

---

pkg/sql/ttl/ttljob/ttljob.go, line 420 at r2 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

hm, obviously this compiled, but a bit confused how since nothing gets returned in this branch.

it's in a for loop so continuously executes until stats is closed

[craig]

Build succeeded:

• GitHub CI (Cockroach)