yaml session atomic write #101

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants

jamhed commented Oct 17, 2012

No description provided.

Contributor

yanick commented Oct 21, 2012

Looks good. I give it my full +1. :-)

Owner

sukria commented Oct 27, 2012

Why is it better than the existing lock mechanism?

-    open my $fh, '>', $session_file or die "Can't open '$session_file': $!\n";
-    flock $fh, LOCK_EX or die "Can't lock file '$session_file': $!\n";
-    set_file_mode($fh);
-    print {$fh} YAML::Any::Dump($self);
-    close $fh or die "Can't close '$session_file': $!\n";
+    atomic_write( setting('session_dir'), yaml_file($self->id), YAML::Dump($self) );
Owner

sukria commented Oct 27, 2012

A rebased version is pushed to review/jamhed/pr/yaml-atomic-write

jamhed commented Oct 27, 2012

On 27.10.2012 20:05, Alexis Sukrieh wrote:

Why is it better than the existing lock mechanism?

  • open my $fh, '>', $session_file or die "Can't open '$session_file': $!\n";
  • flock $fh, LOCK_EX or die "Can't lock file '$session_file': $!\n";
  • set_file_mode($fh);
  • print {$fh} YAML::Any::Dump($self);
  • close $fh or die "Can't close '$session_file': $!\n";
  • atomic_write( setting('session_dir'), yaml_file($self->id), YAML::Dump($self) );


Reply to this email directly or view it on GitHub #101 (comment).

Because I have issues with the existing locking mechanism,
my yaml session files get broken.

Owner

sukria commented Oct 29, 2012

Which issue? Can you elaborate?

I'd like to understand why the locking mechanism is bad here :)

Thanks!

jamhed commented Oct 29, 2012

On 29.10.2012 13:24, Alexis Sukrieh wrote:

Which issue? Can you elaborate?

I'd like to understand why the locking mechanism is bad here :)

I really don't know why, but when I have an application
that writes to a session file after each request running under the starman server I regularly
get the session file broken, like unparseable. When I inspected it I saw it was somehow overwritten,
e.g. some data was appended to the end?
This was quite reproduceable, so I decided to do atomic writes with file move.

Contributor

yanick commented Nov 3, 2012

As I understand it, the original locking mechanism can end up corrupting the yaml file if the process is killed in the middle of the 'print {$fh} ...' statement. The new mechanism would corrupt the file if the process is killed in the middle of the file move done in atomic_write(). The virtue of the latter over the former, I think, is that it's much faster and thus corruption is less likely to happen (and if something happens, you end up with no file at all instead of a truncated one).

jamhed commented Nov 3, 2012

On 03.11.2012 09:47, Yanick Champoux wrote:

As I understand it, the original locking mechanism can end up corrupting the yaml file if the process is killed in the middle of the 'print {$fh} ...' statement. The new mechanism would corrupt the file if the process is killed in the
middle of the file move done in atomic_write(). The virtue of the latter over the former, I think, is that it's much faster and thus corruption is less likely to happen (and if something happens, you end up with no file at all instead of a
truncated one).

No file is definitely better than corrupted one, because if you have a corrupted file
all you'll see is an error page, and the only way to fix this is to delete a
session file or a cookie file in a browser.

Owner

sukria commented Jan 27, 2013

I think this PR is obsolete since we now have Dancer::Core::Role::SessionFactory::File

Feel free to open the same kind of issue/PR if it applies to it.

sukria closed this Jan 27, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment