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

[Maxime Petazzoni]

Hi,

* Thomas Petazzoni <address@hidden> [2010-02-02 22:19:06]:

> >   * 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 ?

Sorry, let me rephrase: I now sort by content modification (st_mtime)
instead of last metadata change (st_ctime).

> >   * 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.
>
> > [snip]
> 
> Keeping the thumbnails means that this loop will everytime check the
> thumbnails... and everytime see that we don't need to remove them.

I know, this is one point I'm not very happy with. We need to rethink
the cleanup strategy. We agree that we clean jobs entirely when we need
to remove one of its while, that's good.

About the thumbnails, I thought keeping them would be nice for archive
purposes (the big list of all maps rendered my MapOSMatic, with
thumbnails, would be a lot nicer). And there's actually a very quick fix
to exclude thumbnails from the list: change

        files = map(lambda f: self.get_file_info(f),
                    [os.path.join(RENDERING_RESULT_PATH, f)
                        for f in os.listdir(RENDERING_RESULT_PATH)
                        if not f.startswith('.')])

by

        files = map(lambda f: self.get_file_info(f),
                    [os.path.join(RENDERING_RESULT_PATH, f)
                        for f in os.listdir(RENDERING_RESULT_PATH)
                        if not (f.startswith('.')] or
                                f.endswidth('_small.png')))

You may be worried about the space occupied by the thumbnails after a while. We
discussed this already, and a single Paris SVG rendering easily takes more disk
space than hundreds of thumbnails. And the 80% of RENDERING_RESULT_MAX_SIZE
foolproofs this, keeping space for thumbnails.

What do you think?

> 
> >      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 ?

Indeed.

> 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.

I don't share this opinion, but two against one, you win :-) I'll make
the changes (they'll apply on top of this series).

> Thanks again for your work!

My pleasure, always fun to code in Python.

- Maxime
-- 
Maxime Petazzoni <http://www.bulix.org>
 ``One by one, the penguins took away my sanity.''
Linux kernel and software developer at MontaVista Software

signature.asc
Description: Digital signature