Re: [Maposmatic-dev] [PATCH maposmatic 1/9] Improve the file cleanup mechanism

[Thomas Petazzoni]

Hello!

Thanks a lot for working on this, it's definitely something we should
fix soonish.

On Sun, 24 Jan 2010 14:43:15 +0100
Maxime Petazzoni <address@hidden> wrote:

> Previously, when the rendering directory was over the defined
> threshold, files where removed progressively, oldest first, to make
> up some space. No information was kept about jobs whose files were
> removed, making it harder to keep track of valid jobs with files
> available.
> 
> This new mechanism brings the following features:
> 
>   * files are now sorted by content modification time and not creation
>     time (a simple chmod could mess up the order);

Are you sure ? Didn't you mix "modification" and "creation" in your
sentence ?

>   * when a file needs to be removed, all files from its parent job are
>     removed, and the job's has_files flag is set to false. It does not
>     make a lot of sense to keep partial renderings anyway (the map
>     without the index? in different formats? Not good.)

True.

>   * thumbnails of jobs in the database are always kept, so we can
>     display what the map looked like even if we don't have the files
>     around anymore;

This is a point of discussion we had earlier on IRC. I'm not sure it's
really worth keeping thumbnails around for renderings for which we
don't have anymore the real files.

> +        # from
> +        f = files.pop()
> +        name = os.path.basename(f[0])
> +        job = MapRenderingJob.objects.get_by_filename(name)
> +        if job:
> +            removed, saved = job.remove_all_files()
> +            size -= saved
> +
> +            # If files were removed, log it. If not, it only means
> only the
> +            # thumbnail remained, and that's good.
> +            if removed:
> +                LOG.info("Removed %d files from job #%d (%s)." %
> +                         (removed, job.id,
> get_formatted_details(saved, size, threshold))) +

Keeping the thumbnails means that this loop will everytime check the
thumbnails... and everytime see that we don't need to remove them.

>      def has_output_files(self):
> +        """Tells if this jobs still has its output files present in
> the
> +        RENDERING_RESULT_PATH. Their actual presence is checked even
> if
> +        has_files is True."""

Isn't has_files a leftover from a previous design ?

Concerning the is_done() function, I have a comment similar to the one
raised by David Mentré.

Shouldn't we keep:

 def is_done():
    return self.status == 2

and introduce something like:

 def is_obsolete():
    return self.status == 3

When a rendering has been removed, it's not really longer "done", in my
opinion.

Thanks again for your work!

Cheers,

Thomas
-- 
Thomas Petazzoni                         http://thomas.enix.org
Promouvoir et défendre le Logiciel Libre http://www.april.org
Logiciels Libres à Toulouse              http://www.toulibre.org

signature.asc
Description: PGP signature