New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

While writing to the output file, lock using LockFile::Simple #6

Merged
merged 3 commits into from Apr 2, 2015

Conversation

Projects
None yet
2 participants
@brummett
Contributor

brummett commented Mar 31, 2015

If the PSGI environment keys 'psgi.multithread' or 'psgi.multiprocess' are
true, lock the output file before writing to avoid writes stomping over
each other.

It uses LockFile::Simple to implement advisory locking on the output file before each write by dropping a *.lock file next to the output file.

I'm not sure I'm happy about the test. It's really too dependent on the implementation of how Recorder writes to the output file and how LockFile::Simple names the lock file.

While writing to the output file, lock using LockFile::Simple
If the PSGI environment keys 'psgi.multithread' or 'psgi.multiprocess' are
true, lock the output file before writing to avoid writes stomping over
each other.
@hoelzro

This comment has been minimized.

Owner

hoelzro commented Mar 31, 2015

@brummett Could we do this with flock instead of LockFile::Simple? That way, not only do we avoid a dependency, but we also don't need to generate a filename to lock on.

@brummett

This comment has been minimized.

Contributor

brummett commented Mar 31, 2015

That's not a problem. Is Scope::Guard ok as a dependency, or should I work around that, too?

@hoelzro

This comment has been minimized.

Owner

hoelzro commented Mar 31, 2015

No, that's ok!

@brummett

This comment has been minimized.

Contributor

brummett commented Mar 31, 2015

Added new commit to use flock() instead of LockFile::Simple

@hoelzro

This comment has been minimized.

Owner

hoelzro commented Mar 31, 2015

Thanks, @brummett! I'll give this a look over sometime today.

@hoelzro

This comment has been minimized.

Owner

hoelzro commented Apr 1, 2015

I don't think I'll get around to really digging into this today or tomorrow; I'm moving apartments right now, so that's taking up a lot of my time. I'll have an in-depth look after I'm settled in!

@brummett

This comment has been minimized.

Contributor

brummett commented Apr 1, 2015

There's no rush - don't worry about it.

@hoelzro

This comment has been minimized.

Owner

hoelzro commented Apr 2, 2015

@brummett I think the logic is sound, so I'm going to going to merge this. Thanks!

hoelzro added a commit that referenced this pull request Apr 2, 2015

Merge pull request #6 from brummett/lock
While writing to the output file, lock using LockFile::Simple

@hoelzro hoelzro merged commit 629def5 into hoelzro:master Apr 2, 2015

@brummett brummett deleted the brummett:lock branch Apr 4, 2015

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