storage/sources: Minor fixes for source table objects

[rjobanp]

Motivation

• This PR fixes a recognized bug. Fixes https://github.com/MaterializeInc/database-issues/issues/8542

Also fixes https://github.com/MaterializeInc/database-issues/issues/8557 and fixes https://github.com/MaterializeInc/database-issues/issues/8545 and fixes https://github.com/MaterializeInc/database-issues/issues/8558

We were error logging whenever source tables were dropped after their corresponding source which was always happening when they are dropped together in a transaction due to the implicit ordering in src/adapter/src/coord/ddl.rs. There isn't an actual bug in dropping out-of-order so I could have just removed the error log but I thought that it would be better to keep it since it might be useful in any future issue around dependency ordering.

The error log happens here:

materialize/src/storage-controller/src/lib.rs

Lines 1453 to 1461 in 03b586b

	// Adjust the source to remove this export.
	let ingestion_state = match self.collections.get_mut(&ingestion_id) {
	Some(ingestion_collection) => ingestion_collection,
	// Primary ingestion already dropped.
	None => {
	tracing::error!("primary source {ingestion_id} seemingly dropped before subsource {id}");
	continue;
	}
	};

Tips for reviewer

@MaterializeInc/testing I wasn't actually able to repro https://github.com/MaterializeInc/database-issues/issues/8542 using the command in that issue but I believe this is the fix for that issue. If you could try to repro on this branch that would be awesome.

Checklist

• This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
• This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
• If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
• If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
• If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

[def-]

This is failing a bunch, but is it possible to create a deterministic test for this?

[rjobanp]

This is failing a bunch, but is it possible to create a deterministic test for this?

@def- what do you mean by 'this'? Do you mean the primary source {ingestion_id} seemingly dropped before subsource {id} error report? Since it is only reported as an error log and is not an actual bug or error I don't know how we can add a test for it. It should be reproducible on main without this PR when doing a DROP SOURCE {} CASCADE for a source with a table attached, since that causes the source to be dropped from the storage controller before the table which is the thing I fixed here

[def-]

I meant a deterministic test for the issue that this PR fixes.

Something like this:

materialize/test/cluster/mzcompose.py

Lines 781 to 786 in 03b586b

	# ensure that an error was put into the logs
	c1 = c.invoke("logs", "clusterd1", capture=True)
	assert (
	"Net-zero records with non-zero accumulation in ReduceAccumulable"
	in c1.stdout
	)

[nrainer-materialize]

Something like this:

materialize/test/cluster/mzcompose.py

Lines 781 to 786 in 03b586b

	# ensure that an error was put into the logs
	c1 = c.invoke("logs", "clusterd1", capture=True)
	assert (
	"Net-zero records with non-zero accumulation in ReduceAccumulable"
	in c1.stdout
	)

Well, we are expecting that this error message no longer occurs.

[rjobanp]

Yes exactly, if this fix works we will no longer see other tests fail when encountering that error message