changefeedccl: add more logging of spans during DistSQL planning

[andyyang890]

This patch adds more logging of spans during various stages of DistSQL
planning to help with debugging plans with erroneous spans.

Epic: None

Release note: None

[blathers-crl]

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[cockroach-teamcity]

This change is 

[rharding6373]

Would it make sense to open a PR with the extra logging on master and then backport it to 23.1 and 23.2? It seems potentially useful for more than just the 23.1 release.

[rharding6373]

I only have a couple minor nits. Thanks for adding more observability!

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @andyyang890 and @jayshrivastava)

---

pkg/ccl/changefeedccl/changefeed_dist.go line 399 at r1 (raw file):

		case rangeDistribution == int64(balancedSimpleDistribution):
			if log.ExpensiveLogEnabled(ctx, 2) {
				log.Infof(ctx, "attempt to rebalance ranges using balanced simple distribution")

minor nit: s/attempt to rebalance/rebalancing

---

pkg/ccl/changefeedccl/changefeed_dist.go line 443 at r1 (raw file):

		for i, sp := range spanPartitions {
			if log.ExpensiveLogEnabled(ctx, 2) {
				log.Infof(ctx, "watched spans for node %d: %s", sp.SQLInstanceID, sp)

This log seems redundant with the previous one. Do you think we need both? I would favor keeping this log in favor of the previous one.

[andyyang890]

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @jayshrivastava and @rharding6373)

---

pkg/ccl/changefeedccl/changefeed_dist.go line 399 at r1 (raw file):

Previously, rharding6373 (Rachael Harding) wrote…

minor nit: s/attempt to rebalance/rebalancing

Done.

---

pkg/ccl/changefeedccl/changefeed_dist.go line 443 at r1 (raw file):

Previously, rharding6373 (Rachael Harding) wrote…

This log seems redundant with the previous one. Do you think we need both? I would favor keeping this log in favor of the previous one.

I think it could still be useful to have both log messages so that we know the spans right after rebalancing (previous log) and the spans right before we pass them to the processors (this log). This way if there are any other immediate transformations, we'll be able to conclusively tell whether rebalancing messed up the spans and/or whether the final spans we're giving to the processors are right. Thoughts?

[rharding6373]

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

---

pkg/ccl/changefeedccl/changefeed_dist.go line 443 at r1 (raw file):

Previously, andyyang890 (Andy Yang) wrote…

I think it could still be useful to have both log messages so that we know the spans right after rebalancing (previous log) and the spans right before we pass them to the processors (this log). This way if there are any other immediate transformations, we'll be able to conclusively tell whether rebalancing messed up the spans and/or whether the final spans we're giving to the processors are right. Thoughts?

Ah, didn't see where they could be modified in between. Seems ok as is.

[andyyang890]

TFTR!

bors r=rharding6373

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)

[blathers-crl]

Encountered an error creating backports. Some common things that can go wrong:

1. The backport branch might have already existed.
2. There was a merge conflict.
3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.

---

error creating merge commit from ad4e8f0 to blathers/backport-release-23.1-119549: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 23.1.x failed. See errors above.

---

error creating merge commit from ad4e8f0 to blathers/backport-release-23.2-119549: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 23.2.x failed. See errors above.

---

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}