[GRASS-dev] r65348 (GRASS_TMPDIR_MAPSET)

Maybe I'm overlooking something, but this change appears to violate
the requirement that the file created by G_tempfile() is on the same
filesystem (partition) as the mapset directory.

Certain files (e.g. the cell/fcell files for rasters) are created via
G_tempfile() then atomically moved into place with rename(), which
requires that both source and destintion to be on the same
filesystem).

--
Glynn Clements <glynn@gclements.plus.com>

On Wed, Jun 3, 2015 at 9:55 AM, Glynn Clements <glynn@gclements.plus.com>
wrote:

Maybe I'm overlooking something, but this change appears to violate
the requirement that the file created by G_tempfile() is on the same
filesystem (partition) as the mapset directory.

Certain files (e.g. the cell/fcell files for rasters) are created via
G_tempfile() then atomically moved into place with rename(), which
requires that both source and destintion to be on the same
filesystem).

This doesn't answer your question but I think that the motivation was
somehow related to #2185.

https://trac.osgeo.org/grass/ticket/2185

Hi,

2015-06-03 15:55 GMT+02:00 Glynn Clements <glynn@gclements.plus.com>:

Maybe I'm overlooking something, but this change appears to violate
the requirement that the file created by G_tempfile() is on the same
filesystem (partition) as the mapset directory.

right, see [1,2].

Certain files (e.g. the cell/fcell files for rasters) are created via
G_tempfile() then atomically moved into place with rename(), which
requires that both source and destintion to be on the same
filesystem).

I have updated G_rename_file() to deal with it [3], do you prefer
another solution?

Martin

[1] https://trac.osgeo.org/grass/ticket/2185
[2] https://github.com/imincik/gis-lab/issues/356
[3] https://trac.osgeo.org/grass/browser/grass/trunk/lib/gis/rename.c#L32

--
Martin Landa
http://geo.fsv.cvut.cz/gwiki/Landa
http://gismentors.cz/mentors/landa

Martin Landa wrote:

I have updated G_rename_file() to deal with it [3], do you prefer
another solution?

Yes. I'd prefer that lib/raster continues to create the file on the
same filesystem as the mapset and atomically rename()s it into place.

IOW, the new tempfile behaviour needs to be availble in addition to
the long-standing behaviour, not a replacement for it.

Whether G_tempfile() itself has the old behaviour or the new behaviour
isn't important. Given that most users of G_tempfile() don't care
where the tempfile goes, making G_tempfile() have the new behaviour
and using a different name for what lib/raster needs is probably less
work.

--
Glynn Clements <glynn@gclements.plus.com>

Hi,

2015-06-03 20:12 GMT+02:00 Glynn Clements <glynn@gclements.plus.com>:

Yes. I'd prefer that lib/raster continues to create the file on the
same filesystem as the mapset and atomically rename()s it into place.

IOW, the new tempfile behaviour needs to be availble in addition to
the long-standing behaviour, not a replacement for it.

this is controlled by GRASS_TMPDIR_MAPSET variable, if not set the
default is the long-standing behaviour.

[...]

Martin

--
Martin Landa
http://geo.fsv.cvut.cz/gwiki/Landa
http://gismentors.cz/mentors/landa

Martin Landa wrote:

> Yes. I'd prefer that lib/raster continues to create the file on the
> same filesystem as the mapset and atomically rename()s it into place.
>
> IOW, the new tempfile behaviour needs to be availble in addition to
> the long-standing behaviour, not a replacement for it.

this is controlled by GRASS_TMPDIR_MAPSET variable, if not set the
default is the long-standing behaviour.

The the temporary files used by lib/raster need to be on the same
filesystem as the mapset regardless of any environment settings.

--
Glynn Clements <glynn@gclements.plus.com>

Hi,

2015-06-04 16:08 GMT+02:00 Glynn Clements <glynn@gclements.plus.com>:

The the temporary files used by lib/raster need to be on the same
filesystem as the mapset regardless of any environment settings.

please could you write us why? I didn't find any problem (bearing in
mind changes in G_rename_file() which tries to rename file and if
fails copy & remove). Anyway, I wonder how to treat raster lib and
vector lib differently. Martin

--
Martin Landa
http://geo.fsv.cvut.cz/gwiki/Landa
http://gismentors.cz/mentors/landa

Martin Landa wrote:

2015-06-04 16:08 GMT+02:00 Glynn Clements <glynn@gclements.plus.com>:

> The the temporary files used by lib/raster need to be on the same
> filesystem as the mapset regardless of any environment settings.

please could you write us why?

So that they can be rename()d into place.

Copy-and-delete isn't remotely the same thing. Aside from the
gratuitous performance hit, it creates a window where the cell/fcell
file is invalid.

[Technically there's already a window because we remove() the file
first because Windows doesn't support atomic rename(), but that's
minuscule compared to the time taken to copy a file.]

On that note, G_rename() should be called e.g. G_rename_or_copy() or
G_move_file() or something, to avoid misleading anyone. A "rename" has
specific semantics which G_rename() doesn't provide (e.g. preservation
of hard links, permissions, ownership, extended attributes, inode
number, holes, open descriptors, ... everything except the path, in
fact).

I didn't find any problem (bearing in
mind changes in G_rename_file() which tries to rename file and if
fails copy & remove). Anyway, I wonder how to treat raster lib and
vector lib differently. Martin

I don't know what the vector library's requirements are, but any
temporary file which is intended to be moved to a specific location
should be created such that it can be rename()d to that location (i.e.
on the same filesytem).

The new behaviour is fine for code which wants a temporary file and
doesn't care where it goes (e.g. because it will never be moved).

But there's no justification for the previous behaviour being removed.

--
Glynn Clements <glynn@gclements.plus.com>