Changeset 41014

Timestamp:

Oct 31, 2019, 11:57:27 AM (7 years ago)

Author:

eugene

Message:

replicate_object should pass dest_vol_name to parse_neb_key so they hard_volume status is correctly assessed; treat find_invalid as a boolean (previously it was treated as true if defined even as 0); if find_instaces is passed find_invalid = TRUE, the ensure this is passed to nested calls to find_instance

File:

• branches/eam_branches/ipp-20191011/Nebulous-Server/lib/Nebulous/Server.pm (modified) (17 diffs)

branches/eam_branches/ipp-20191011/Nebulous-Server/lib/Nebulous/Server.pm

@@ -12 +12 @@
 
 use base qw( Class::Accessor::Fast );
+
+use Carp;
 
 use Cache::Memcached;
@@ -723 +725 @@
         {
             type        => SCALAR|UNDEF,
-#            callbacks   => {
-#                # check that the volume name requested is valid
-#                'is valid volume name' => sub {
-#                    return 1 if not defined $_[0];
-#                    $self->_is_valid_volume_name($_[0])
-#                },
-#            },
             optional    => 1,
         },
@@ -737 +732 @@
 
     # if a volume name is explicity specified then we should make the
-    # replication onto that volume (even if there is alread an instance on that
+    # replication onto that volume (even if there is already an instance on that
     # volume) if at all possible and throw an error if we can not.
+
     # if a volume name IS NOT specified then we should make the replication
     # onto any (hopefully the best) volume that DOES NOT already have an
-    # instance on it.  If all avilable volume already have an instance on them
+    # instance on it.  If all available volumes already have an instance on them
     # then we should throw an error 
+
     # volume names implied as part of the key are *IGNORED* as the source and
     # *SHOULD NOT* be used as the destination either
 
     eval {
-        $key = parse_neb_key($key);
+        # EAM 2019.10.31 : if I supply dest_vol_name here, then
+        # $key->volume will point at that volume and the section below
+        # will correctly be testing the validity of dest_vol_name
+        $key = parse_neb_key($key, $dest_vol_name);
     };
     $log->logdie("$@") if $@;
@@ -753 +753 @@
     my $db  = $self->db($key);
 
+    ## Old comment:
     # puke if the source volume is bogus, we may want to actually use this as
     # the instance to be copied later
+
+    # EAM 2019.10.31 : in the past, the key-implied volume was
+    # supplied here, which was inconsistent since above it says the
+    # key-implied volume is ignored. By supplying $dest_vol_name to
+    # parse_neb_key above, the key->volume now refers to the
+    # dest_volume
     if (defined $key->volume and not $self->_is_valid_volume_name($key, $key->volume)) {
         unless ($key->hard_volume) {
@@ -762 +769 @@
         }
     }
+
     # puke if the destination volume is bogus
-    if (defined $dest_vol_name
-        and not $self->_is_valid_volume_name($key, $dest_vol_name)) {
-           $log->logdie($key->volume . " is not a valid volume name");
-    }
+    ## XXX if I supply dest_vol_name to parse_neb_key above, this section below is redundant with the above
+    ## if (defined $dest_vol_name
+    ##     and not $self->_is_valid_volume_name($key, $dest_vol_name)) {
+    ##        $log->logdie($key->volume . " is not a valid volume name");
+    ## }
 
     $get_storage_volume_calls = 0;
@@ -772 +781 @@
     my ($vol_id, $vol_host, $vol_path, $vol_xattr);
     if (defined $dest_vol_name) {
+        # if we supplied dest_vol_name, then the call to parse_neb_key above
+        # will have set $key->volume to that value and hard_volume to match
         ($vol_id, $vol_host, $vol_path, $vol_xattr)
-            = $self->_get_storage_volume($key, $dest_vol_name);
+            = $self->_get_storage_volume($key, $key->volume, $key->hard_volume);
     } else {
         ($vol_id, $vol_host, $vol_path, $vol_xattr)
             = $self->_get_replication_volume($key);
     }
+
+    # print "selected target: $vol_id, $vol_host\n";
 
     my $uri;
@@ -1789 +1802 @@
         {
             type        => SCALAR|UNDEF,
-#            callbacks   => {
-#                # check that the volume name requested is valid
-#                'is valid volume name' => sub {
-#                    return 1 if not defined $_[0];
-#                    $self->_is_valid_volume_name($_[0])
-#                },
-#            },
             optional    => 1,
         },
@@ -1807 +1813 @@
     my $sql = $self->sql;
 
-#    unless ($key) {
-#        $log->warn("key was undefined after validate_pos(), trying again...");
-#        return $self->find_instances(@_);
-#    }
+    # default value for find_invalid is false
+    if (not defined $find_invalid) { $find_invalid = 0; }
 
     # vol_name overrides the key implied volume
@@ -1848 +1852 @@
             my $rows;
             # ext_id, available
-            if (defined($find_invalid)) {
+            if (defined($find_invalid) and $find_invalid) {
                 # XXX returns instances which are NOT available
                 $rows = $query->execute($key->path, 0);
@@ -1903 +1907 @@
         {
             type        => SCALAR|UNDEF,
-#            callbacks   => {
-#                # check that the volume name requested is valid
-#                'is valid volume name' => sub {
-#                    return 1 if not defined $_[0];
-#                    $self->_is_valid_volume_name($_[0])
-#                },
-#            },
             optional    => 1,
         },
@@ -1920 +1917 @@
     my $sql = $self->sql;
 
-#    unless ($key) {
-#        $log->warn("key was undefined after validate_pos(), trying again...");
-#        return $self->find_instances(@_);
-#    }
+    if (not defined $find_invalid) { $find_invalid = 0; }
 
     # vol_name overrides the key implied volume
@@ -1991 +1985 @@
             # host_vol_id host_cab_id host_site_id ext_id available
 
-            ### XXX EAM if find_invalid is true, the last arguement here should be 0
-            my $rows = $query->execute($h_vol_id, $h_cab_id, $h_site_id, $key->path, 1);
+            my $rows;
+            if (defined($find_invalid) and $find_invalid) {
+                # returns instances which are NOT available (volume not available)
+                $rows = $query->execute($h_vol_id, $h_cab_id, $h_site_id, $key->path, 0);
+            } else {
+                $rows = $query->execute($h_vol_id, $h_cab_id, $h_site_id, $key->path, 1);
+            }           
             unless ($rows > 0) {
                 $query->finish;
@@ -1999 +1998 @@
         } else {
             $query = $db->prepare_cached( $sql->get_object_instances );
+            # ext_id, available
+
             my $rows;
-            # ext_id, available
-            if (defined($find_invalid)) {
+            if (defined($find_invalid) and $find_invalid) {
                 # returns instances which are NOT available (volume not available)
                 $rows = $query->execute($key->path, 0);
-            }
-            else {
+            } else {
                 $rows = $query->execute($key->path, 1);
             }
@@ -2020 +2019 @@
             my $instance = $row->{ 'uri' };
             push @locations, $instance if $instance;
+
+            # Carp::carp ("instance 2: $row->{ 'uri' }, $row->{ 'vol_id' }, $row->{ 'cab_id' }, $row->{ 'vol_idx' }\n");
         }
     };
@@ -2027 +2028 @@
         if (defined $vol_name and not defined $key->hard_volume) {
             $log->debug("retrying with 'any' volume");
-            return $self->find_instances($key->path, 'any');
+            return $self->find_instances($key->path, 'any', $find_invalid);
         }
         $log->logdie("database error: $@");
@@ -2463 +2464 @@
             # %free, name, avaiable, allocate
             $rows = $query->execute($max_used_space, $name, 1, 1);
-            # XXX destinguish between non-existant and unavailable
+            # XXX distinguish between non-existant and unavailable
             unless ($rows > 0) {
                 $query->finish;
@@ -2572 +2573 @@
 
         $rows = $query->execute($key->path, $max_used_space, 1, 1, $forbidden_cabinet, $forbidden_site, $topfew_count);
-        # XXX destinguish between non-existant and unavailable
+        # XXX distinguish between non-existant and unavailable
         unless ($rows > 0) {
             $query->finish;