Changeset 1322

Timestamp:

10/17/09 08:28:02 (6 weeks ago)

Author:

dormando

Message:

Fix create_close race condition

If two create_close requests are send, and one reads the tempfile, writes the
file row to the db, then gets hung before removing the tempfile... The second
one could potentially trigger its own eventual deletion.

From ptr <pipegeek@…>

Location:

trunk/server/lib/MogileFS

Files:

• Store.pm (modified) (1 diff)
• Worker/Query.pm (modified) (4 diffs)

trunk/server/lib/MogileFS/Store.pm

@@ -1023 +1023 @@
 sub delete_tempfile_row {
     my ($self, $fidid) = @_;
-    $self->dbh->do("DELETE FROM tempfile WHERE fid=?", undef, $fidid);
+    my $rv = $self->dbh->do("DELETE FROM tempfile WHERE fid=?", undef, $fidid);
     $self->condthrow;
+    return $rv;
+}
+
+# Load the specified tempfile, then delete it.  If we succeed, we were
+# here first; otherwise, someone else beat us here (and we return undef)
+sub delete_and_return_tempfile_row {
+    my ($self, $fidid) = @_;
+    my $rv = $self->tempfile_row_from_fid($fidid);
+    my $rows_deleted = $self->delete_tempfile_row($fidid);
+    return $rv if ($rows_deleted > 0);
 }
 

trunk/server/lib/MogileFS/Worker/Query.pm

@@ -366 +366 @@
     my $sto = Mgd::get_store();
 
-    # find the temp file we're closing and making real
-    my $trow = $sto->tempfile_row_from_fid($fidid) or
+    # find the temp file we're closing and making real.  If another worker
+    # already has it, bail out---the client closed it twice.
+    my $trow = $sto->delete_and_return_tempfile_row($fidid) or
         return $self->err_line("no_temp_file");
 
@@ -381 +382 @@
     my $old_fid = MogileFS::FID->new_from_dmid_and_key($dmid, $key);
     if ($old_fid) {
+        # Fail if a file already exists for this fid.  Should never
+        # happen, as it should not be possible to close a file twice.
+        return $self->err_line("fid_exists")
+            unless $old_fid->{fidid} != $fidid;
+
         $old_fid->delete;
     }
@@ -412 +418 @@
     # mark it as needing replicating:
     $fid->enqueue_for_replication(from_device => $devid);
-    $sto->delete_tempfile_row($fidid);
 
     if ($fid->update_devcount) {
@@ -1546 +1551 @@
         'no_ip' => "IP required to create host",
         'no_port' => "Port required to create host",
+        'no_temp_file' => "No tempfile or file already closed",
         'none_match' => "No keys match that pattern and after-value (if any).",
         'plugin_aborted' => "Action aborted by plugin",