rangedesc: add lazy iterator

[dt]

see commits.

[cockroach-teamcity]

This change is 

[arulajmani]

modulo comments.

Reviewed 4 of 4 files at r1, 2 of 2 files at r2, 3 of 3 files at r3, 1 of 1 files at r4, 2 of 2 files at r5, 1 of 1 files at r6, 2 of 2 files at r7, 2 of 2 files at r8, 1 of 1 files at r9, 4 of 4 files at r10, 1 of 1 files at r11, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @andrewbaptist, @dt, and @msbutler)

---

pkg/server/node.go line 2574 at r6 (raw file):

	const pageSize = 100

	iter, err := n.execCfg.RangeDescIteratorFactory.NewLazyIterator(stream.Context(), args.Span, 100)

nit: s/100/pageSize/g

---

pkg/util/rangedesc/rangedesc.go line 86 at r11 (raw file):

	// constructor fetches the entirety of the result set and the returned iter is
	// merely iterating over that buffered result set, so this is actually better
	// treated as if it is a batch API, eg GetAllRangeDescs(span). Callers should

After this comment was initially written, we spoke on Slack about NewLazyIterator giving an inconsistent view of range descriptors to callers, unlike NewIterator, which provides a consistent view. Some callers (notably the observability related ones) need a consistent view.

Consider softening the language in this comment to dissuade users from using this function entirely; instead, maybe let's add some words about the consistent/inconsistent nature of both these methods, and let callers decide what's appropriate for them.

[dt]

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @andrewbaptist, @arulajmani, and @msbutler)

---

pkg/server/node.go line 2574 at r6 (raw file):

Previously, arulajmani (Arul Ajmani) wrote…

nit: s/100/pageSize/g

I think this is a stale revision since I switched this to pageSize back on dec 15 (oh reviewable).

---

pkg/util/rangedesc/rangedesc.go line 86 at r11 (raw file):

Previously, arulajmani (Arul Ajmani) wrote…

After this comment was initially written, we spoke on Slack about NewLazyIterator giving an inconsistent view of range descriptors to callers, unlike NewIterator, which provides a consistent view. Some callers (notably the observability related ones) need a consistent view.

Consider softening the language in this comment to dissuade users from using this function entirely; instead, maybe let's add some words about the consistent/inconsistent nature of both these methods, and let callers decide what's appropriate for them.

Softened it up a bit. I might just go add an asOf timestamp to the lazy iterator constructor and make it scan batches at a fixed timestamp so we can just retire the eager iterator instead, but I've softer the language for now until I do that.

[dt]

TFTR!

bors r+

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)