GNOME Bugzilla – Bug 728775
Failed thumbnail creation doesn't unlink tmp_path
Last modified: 2014-05-05 09:31:00 UTC
If for instance a user is over-quota, all thumbnail creation fails. We store user home dirs in an EMC-CELERRA NAS exporting it over NFS. In this system it's possible to create an empty file if the user is over quota, but it fails when gnome try to write the thumbnail pixmap. I see the commit https://git.gnome.org/browse/gnome-desktop/commit/libgnome-desktop/gnome-desktop-thumbnail.c?id=715b3ae32aa4dc8deaaa48138d4411a8393d25e1 that unlink the temporary file when the thumbnail's creation fails, but it only do it in the "normal" folder. It's necessary to unlink also in the "fail" folder. We saw thousands of temporary empty thumbnails in the "fail" folder in all users that are over quota. The patch may be @1719 like this: if (saved_ok) { g_chmod (tmp_path, 0600); g_rename(tmp_path, path); } + else + { + g_unlink(tmp_path); + } Also it may be nice to have control over thumbnail creation threads if all thumbnails fails because nautilus waste many cpu trying to create thumbnails in an endless loop.
Thanks for reporting this! If you have time, attaching a git-formatted patch is welcome, see https://wiki.gnome.org/Git/Developers for more information.
(In reply to comment #0) > If for instance a user is over-quota, all thumbnail creation fails. We store > user home dirs in an EMC-CELERRA NAS exporting it over NFS. In this system it's > possible to create an empty file if the user is over quota, but it fails when > gnome try to write the thumbnail pixmap. > > I see the commit > https://git.gnome.org/browse/gnome-desktop/commit/libgnome-desktop/gnome-desktop-thumbnail.c?id=715b3ae32aa4dc8deaaa48138d4411a8393d25e1 > that unlink the temporary file when the thumbnail's creation fails, but it > only do it in the "normal" folder. > > It's necessary to unlink also in the "fail" folder. We saw thousands of > temporary empty thumbnails in the "fail" folder in all users that are over > quota. No, it's not. The point of creating empty files in the fail folder is so that thumbnailing isn't attempted again for that file. Not having a file in the failed folder would make it try to generate the thumbnail again and again. > Also it may be nice to have control over thumbnail creation threads if all > thumbnails fails because nautilus waste many cpu trying to create thumbnails in > an endless loop. You'll need to file a bug against nautilus about that.
(In reply to comment #2) > (In reply to comment #0) > > If for instance a user is over-quota, all thumbnail creation fails. We store > > user home dirs in an EMC-CELERRA NAS exporting it over NFS. In this system it's > > possible to create an empty file if the user is over quota, but it fails when > > gnome try to write the thumbnail pixmap. > > > > I see the commit > > https://git.gnome.org/browse/gnome-desktop/commit/libgnome-desktop/gnome-desktop-thumbnail.c?id=715b3ae32aa4dc8deaaa48138d4411a8393d25e1 > > that unlink the temporary file when the thumbnail's creation fails, but it > > only do it in the "normal" folder. > > > > It's necessary to unlink also in the "fail" folder. We saw thousands of > > temporary empty thumbnails in the "fail" folder in all users that are over > > quota. > > No, it's not. The point of creating empty files in the fail folder is so that > thumbnailing isn't attempted again for that file. Not having a file in the > failed folder would make it try to generate the thumbnail again and again. > Well. If so I think that there is also a bug in te code in function gnome_desktop_thumbnail_factory_create_failed_thumbnail. Te code @1706 is: saved_ok = gdk_pixbuf_save (pixbuf, tmp_path, "png", NULL, "tEXt::Thumb::URI", uri, "tEXt::Thumb::MTime", mtime_str, "tEXt::Software", "GNOME::ThumbnailFactory", NULL); g_object_unref (pixbuf); if (saved_ok) { g_chmod (tmp_path, 0600); g_rename(tmp_path, path); } If gdk_pixbuf_save fail in over quota, the temp name stays and a temp file is created again in an endless loop. It may call g_chmod and g_rename if state is saved_ok or not. The patch maybe to remove the "if" sentence. If not, as you say, the thumbnail is generated again and again. Also I don't see any sense saving the pixmap of a failed thumbnail. It's enough to create a empty thumbnail file. > > Also it may be nice to have control over thumbnail creation threads if all > > thumbnails fails because nautilus waste many cpu trying to create thumbnails in > > an endless loop. > > You'll need to file a bug against nautilus about that. The bug isn't for nautilus because nautilus only calls libgnomedesktop functions. All code fo thumbnail creation is inside libgnomedesktop library, but the top command reports cpu usage in nautilus that is the application that are calling these functions of course.
Created attachment 275865 [details] [review] Proposed patch for this bug This patch rename ALWAYS the tmp_path in case of failed thumbnail creation to address this problem.
Created attachment 275877 [details] [review] thumbnailer: Try harder to create a failed thumbnail If a failed thumbnail is created because the pixbuf fail to save (for instance if user is over quota) we should still try to rename the temporary file that might have been created. If not, the thumbnail will not be marked as failed, and thumbnailing will be reattempted.
Attachment 275877 [details] pushed as 54f68ab - thumbnailer: Try harder to create a failed thumbnail