fix: actually drop delta tables

[universalmind303]

closes #1870

I wanted to open this up to make sure that others are aligned on the direction I took with this before getting too far. This works as is, but could use a bit of cleanup in areas.

It uses our scheduler to spawn a system_exec to remove the delta tables. Currently it just logs the output, but we could put them in a table or something else.. Logging seemed to be fine for now.. I didn't want to over-engineer and throw away a bunch of work if this isn't the direction we want to take.

[universalmind303]

so this is technically working fine, but it isn't working inside of a test, likely because of the known issues with the scheduler. The task gets scheduled, but never picked up. Even when putting a long sleep in the test, the task never gets picked up. So since we can't write an integration test for this, I'll keep this in a WIP until we can get our scheduler working properly

[tychoish]

so this is technically working fine, but it isn't working inside of a test, likely because of the known issues with the scheduler. The task gets scheduled, but never picked up. Even when putting a long sleep in the test, the task never gets picked up. So since we can't write an integration test for this, I'll keep this in a WIP until we can get our scheduler working properly

When you say technically working fine, you mean that you can observe it working correctly in manual testing but that because the distributed execution scheduler is not working in the testing environment that your tests don't pass?

I think keeping it draft until distexec gets more ironed out seems fine. It also doesn't seem necessary to use the distexec scheduler for this because:

• why not just have the remove operation run in the calling thread? having the calling operation return before the delete operation returns seems like it could lead to some confusions (and failed deletes)
• why not spawn a task for doing this directly? The scheduler mostly exists to provide fairness and limitations when we hav a lot of very small tasks (as we would in a dist-exec situation,) but that doesn't seem like a huge problem here.
• using the distexec scheduler for the "non-dist-exec" code paths, seems potentially confusing, and might make it harder operationally to turn things off, or understand things.

Having said that, all of the potential bugs or issues from using the distexec scheduler before everything becomes distexec is just that we'd regress on this issue, which is already happening today, so I wouldn't worry about it too hard, though obviously this is all moot until the scheduler works better.

[universalmind303]

When you say technically working fine, you mean that you can observe it working correctly in manual testing but that because the distributed execution scheduler is not working in the testing environment that your tests don't pass?

yeah, it functionally works, but not in the testing suite.

[scsmithr]

If we have table entries, do we need table references? Can we just use the info provided in entries?

[universalmind303]

I originally wanted to go that route, but we don't have access to an entry resolver inside the execution context, only during planning.

[tychoish]

seems reasonable, one question.