[DNM] Add documentation for hypothetical pre-timeout scripts

[benesch]

In the spirit of documentation driven development, this commit adds docs for the pre-timeout scripts proposed in #1906.

I'll start prototyping an implementation soon. In the meantime, I figured we could start shaking out the major design questions by reviewing what's proposed in the documentation here.

cc @criccomini

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 79.93%. Comparing base (eaa5634) to head (a107acf). Report is 13 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2018      +/-   ##
==========================================
- Coverage   79.94%   79.93%   -0.01%     
==========================================
  Files          97       97              
  Lines       23631    23677      +46     
==========================================
+ Hits        18891    18926      +35     
- Misses       4740     4751      +11     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[benesch]

Thanks for this great feedback, @sunshowers! Will take an another spin through the docs and start working in answers to the questions you mentioned. I've got some long haul flights coming up this week and next, and this should make for a perfect plane project.

[sunshowers]

Thanks for the wait! I've gone through and answered all the questions. I think the design is in generally good shape, and you're asking all the right questions :D

[sunshowers]

Thanks! FYI this is on my queue to look at this weekend -- have a lot of stuff at work this week. I like the pretimeout state :)

It's currently ok (I was expecting the impl to be this large), but I think before landing it would be good to get into a series of smaller commits.

[benesch]

FYI this is on my queue to look at this weekend -- have a lot of stuff at work this week.

No rush on my end at all—take as long as you need!

I like the pretimeout state :)

🎉 !

It's currently ok (I was expecting the impl to be this large), but I think before landing it would be good to get into a series of smaller commits.

Sounds good! I'm happy to polish up the commit series whenever works best for you.

[sunshowers]

Still in the queue to review -- higher priority stuff came up but I'm hoping to carve out some time this weekend.

[benesch]

No problem at all, @sunshowers, and thanks for the update!

[benesch]

@sunshowers no pressure, but just wanted to check in here!

[benesch]

My turn to be a bit behind: busy starting a new job! Hoping to have time to get back to this shortly but almost certainly not before the end of the week.

[sunshowers]

I know things have been a bit behind here, but wanted to let you know that I ended up using a lot of this implementation for the new wrapper script support. So thank you for implementing this!

Hopefully this implementation becomes relatively small as a result.

[benesch]

Oh, that's great to hear, @sunshowers! Sorry that I wound up being too busy to push this over the finish line.

[benesch]

I actually wonder if wrapper scripts are all we need here? For SlateDB, the wrapper script could be responsible for handling the SIGTERM and invoking gdb before exiting.

[sunshowers]

Yeah wrapper scripts can definitely fulfill this role! But ease of use is an important consideration:

• Nextest manages timeouts but has no clear way of communicating that to the wrapper script
• Writing a shell script that does this with a trap instruction is possible but not easy
• There isn't a good story on Windows
• Wrapper scripts are a power tool that are easy to get wrong

So overall, explicit support for pre-timeout scripts is still valuable.

[benesch]

• Writing a shell script that does this with a trap instruction is possible but not easy
• Wrapper scripts are a power tool that are easy to get wrong

Indeed! I rigged this up for SlateDB just now and it is a bit fiddly: https://github.com/slatedb/slatedb/pull/363/files#diff-8b5d6d952cf5fddb1cedc6bb8c4aebd2048ac395f30b43e80f082d7d23a58d64

So overall, explicit support for pre-timeout scripts is still valuable.

Makes sense. Realistically I don't think I'm going to be able to make the time to continue this work in the coming months. Is it worth leaving the PR open in case you or someone else has time to pick it up? Or better to just close it out until someone else needs it?

[sunshowers]

Let's leave this open, I'll try and finish it up some time soon if you don't get to it. I feel like around 50% of this landed already.

[benesch]

Sounds good—thanks, @sunshowers!