Data not deleted from /var/tmp

EIDA / mediatorws

EIDA NG Mediator/Federator web services

GNU General Public License v3.0

6 stars 6 forks source link

Data not deleted from /var/tmp #67

Closed Jollyfant closed 5 years ago

Jollyfant commented 5 years ago

Hi, it looks like data files are not deleted from /var/tmp/ when the request is aborted. Our disk filled up with 300GB of data! At least that means it was being tested :D

damb commented 5 years ago

Thanks @Jollyfant,

this is not a bug. In case of errors (i.e. HTTP status codes of endpoint request not in (200, 413)) eida-federator does not remove temporary files. The current implementation exclusively logs a WARNING:

https://github.com/EIDA/mediatorws/blob/46304c2354c5337c89f042fa9b914309aea49e26/eidangservices/federator/server/process.py#L254-L255

Due to development purposes I implemented it this way in order to be able to easier check for errors. Which approach do you purpose/prefer?:

Stick with the current approach and install a cron job with e.g. tmpreaper removing (outdated) files periodically (Question: What means outdated for long running requests?).
Remove temporary files even on errors?
Probably the most flexible/safest solution: eida-federator removes outdated temporary files after some (configurable) time.

kaestli commented 5 years ago

one more option: have "Remove temporary files even on errors = true|false" as a configuration option.

Jollyfant commented 5 years ago

Remove temporary files even on errors?

I think this a good idea. No reason to keep them around if the client hangs up or there's an error. Also there could be a (non-federator) clean-up script that runs daily. But I prefer to have the Federator clean what it writes.

damb commented 5 years ago

I'm going to implement the solution suggested by @kaestli.

damb commented 5 years ago

@Jollyfant, is it ok for you if I'll assign the issue to myself? Thanks and Cheers.

Jollyfant commented 5 years ago

Yeah no problem I think I accidentally assigned me.

damb commented 5 years ago

Should be fixed with 2801d8e37a1b97c10c79b67530cb31391e9213a1.

Note, that for fdsnws-station?format=xml still under certain race conditions temporary files might remain. StationXMLNetworkCombinerTask internally uses a multiprocessing.Pool which appears tasks being actually force killed when executing pool.terminate(). The cleanest approach would have been implementing something such as pool.shutdown() which

would cancel pending tasks
would wait until tasks currently executed return
would finalize the termination process.

However, I did not go for that, yet.