sql: fix flakiness with InternalExecutorFactoryMemMonitor closing TestServers

[RichardJCai]

Release note: None

[cockroach-teamcity]

This change is 

[ajwerner]

Reviewed all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @RichardJCai and @yuzefovich)

---

pkg/server/server_sql.go line 934 at r1 (raw file):

	// systemIeMonitor is used for the jobsIeFactory and for the
	// circularInternalExecutor.
	// The schema changer may still be executing jobs queries.

Can you help me understand how this happens? Shouldn't the schema changer queries stop before the Closer gets called?

[RichardJCai]

Reviewed all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @RichardJCai and @yuzefovich)

pkg/server/server_sql.go line 934 at r1 (raw file):

	// systemIeMonitor is used for the jobsIeFactory and for the
	// circularInternalExecutor.
	// The schema changer may still be executing jobs queries.

Can you help me understand how this happens? Shouldn't the schema changer queries stop before the Closer gets called?

I just updated the PR to just not close anything.

I was seeing in TestAtMostOneRunningCreateStats that the monitor I attached to the jobs internal executor was still allocated some memory when Closer was called. Is it possible that the query stops but theres a race with the memory being cleared.

[yuzefovich]

Reviewed 1 of 3 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner)

[RichardJCai]

Thanks!

bors r=yuzefovich

[ajwerner]

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

---

pkg/server/server_sql.go line 934 at r1 (raw file):

Previously, RichardJCai (Richard Cai) wrote…

I just updated the PR to just not close anything.

I was seeing in TestAtMostOneRunningCreateStats that the monitor I attached to the jobs internal executor was still allocated some memory when Closer was called. Is it possible that the query stops but theres a race with the memory being cleared.

🤔 that's not very satisfying. Do you have more details on what was not released? If we could get to the bottom of it, it'd be really nice. It'd give us a claim that we do actually clean up properly. If we don't then we'll just let more folks get away with not cleaning up properly.

[craig]

Build succeeded:

• GitHub CI (Cockroach)

[RichardJCai]

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

pkg/server/server_sql.go line 934 at r1 (raw file):

Previously, RichardJCai (Richard Cai) wrote…
🤔 that's not very satisfying. Do you have more details on what was not released? If we could get to the bottom of it, it'd be really nice. It'd give us a claim that we do actually clean up properly. If we don't then we'll just let more folks get away with not cleaning up properly.

Opened up an issue with context for now - #84302 don't have much time in the near future to dig into this.