Notifying instead of failing fixes #2111

app/models/table.rb

@@ -97,16 +97,28 @@ def default_privacy_values
 
   def geometry_types
     if schema.select { |key, value| key == :the_geom }.length > 0
-      owner.in_database[ %Q{
+      query_geometry_types
+    else
+      []
+    end
+  end
+
+  def query_geometry_types
+    owner.in_database[ %Q{
       SELECT DISTINCT ST_GeometryType(the_geom) FROM (
         SELECT the_geom
         FROM "#{self.name}"
         WHERE (the_geom is not null) LIMIT 10
       ) as foo
     }].all.map {|r| r[:st_geometrytype] }
-    else
-      []
-    end
+  end
+
+  def calculate_the_geom_type
+    return self.the_geom_type if self.the_geom_type.present?
+
+    calculated = query_geometry_types.first
+    calculated = calculated.present? ? calculated.downcase.sub('st_', '') : DEFAULT_THE_GEOM_TYPE
+    self.the_geom_type = calculated
   end
 
   def is_raster?
@@ -144,7 +156,7 @@ def is_raster?
                 :import_from_query,
                 :import_from_table_copy,
                 :importing_encoding,
-                :temporal_the_geom_type,
+                :the_geom_type_value,
                 :migrate_existing_table,
                 # this flag is used to register table changes only without doing operations on in the database
                 # for example when the table is renamed or created. For remove see keep_user_database_table
@@ -527,10 +539,6 @@ def before_create
       else
         create_table_in_database!
         set_table_id
-        unless self.temporal_the_geom_type.blank?
-          self.the_geom_type = self.temporal_the_geom_type
-          self.temporal_the_geom_type = nil
-        end
         set_the_geom_column!(self.the_geom_type)
       end
     end
@@ -547,7 +555,6 @@ def after_create
     set_default_table_privacy
 
     @force_schema = nil
-    $tables_metadata.hset key, 'user_id', user_id
     self.new_table = true
 
     # finally, close off the data import
@@ -580,7 +587,7 @@ def after_save
     manager.set_from_table_privacy(privacy)
     manager.propagate_to(table_visualization)
     if privacy_changed?
-      manager.propagate_to_redis_and_varnish
+      manager.propagate_to_varnish
       update_cdb_tablemetadata
     end
 
@@ -610,7 +617,6 @@ def handle_creation_error(e)
     # Remove the table, except if it already exists
     unless self.name.blank? || e.message =~ /relation .* already exists/
       @data_import.log.append ("Import ERROR: Dropping table #{qualified_table_name}") if @data_import
-      $tables_metadata.del key
 
       self.remove_table_from_user_database unless keep_user_database_table
     end
@@ -668,7 +674,6 @@ def after_destroy
     super
     # Delete visualization BEFORE deleting metadata, or named map won't be destroyed properly
     @table_visualization.delete(from_table_deletion=true) if @table_visualization
-    $tables_metadata.del key
     Tag.filter(:user_id => user_id, :table_id => id).delete
     remove_table_from_stats
     invalidate_varnish_cache
@@ -835,18 +840,6 @@ def privacy_changed?
     previous_changes.keys.include?(:privacy)
   end #privacy_changed?
 
-  def key
-    Table.key(owner.database_name, "#{owner.database_schema}.#{name}")
-  rescue
-    nil
-  end
-
-  # @param db_name String
-  # @param table_name String Must come fully qualified
-  def self.key(db_name, table_name)
-    "rails:#{db_name}:#{table_name}"
-  end
-
   def sequel
     owner.in_database.from(sequel_qualified_table_name)
   end
@@ -886,6 +879,9 @@ def schema(options = {})
     last_columns      = []
     owner.in_database.schema(name, options.slice(:reload).merge(schema: owner.database_schema)).each do |column|
       next if column[0] == THE_GEOM_WEBMERCATOR
+
+      calculate_the_geom_type if column[0] == :the_geom
+
       col_db_type = column[1][:db_type].starts_with?('geometry') ? 'geometry' : column[1][:db_type]
       col = [
         column[0],
@@ -1200,41 +1196,24 @@ def georeference_from!(options = {})
     end
   end
 
-
   def the_geom_type
-    $tables_metadata.hget(key,'the_geom_type') || DEFAULT_THE_GEOM_TYPE
-  rescue => e
-    # FIXME: patch for Redis timeout errors, should be removed when the problem is solved
-    CartoDB::notify_error("Redis timeout error patched", error_info: "Table: #{self.name}. Connection: #{self.redis_connection_info}. Will retry.\n #{e.message} #{e.backtrace.join}")
-    begin
-      $tables_metadata.hget(key,'the_geom_type') || DEFAULT_THE_GEOM_TYPE
-    rescue => e
-      CartoDB::notify_error("Redis timeout error patched retry", error_info: "Table: #{self.name}. Connection: #{self.redis_connection_info}. Second error, won't retry.\n #{e.message} #{e.backtrace.join}")
-      raise e
-    end
-  end
-
-  def redis_connection_info
-    "#{$tables_metadata.client.host}:#{$tables_metadata.client.port}, db: #{$tables_metadata.client.db}, timeout: #{$tables_metadata.client.timeout}"
+    self.the_geom_type_value
   end
 
   def the_geom_type=(value)
-    the_geom_type_value = case value.downcase
+    self.the_geom_type_value = case value.downcase
       when 'geometry'
         'geometry'
       when 'point'
         'point'
       when 'line'
         'multilinestring'
+      when 'multipoint'
+        'point'
       else
         value !~ /^multi/ ? "multi#{value.downcase}" : value.downcase
     end
-    raise CartoDB::InvalidGeomType unless CartoDB::VALID_GEOMETRY_TYPES.include?(the_geom_type_value)
-    if owner.in_database.table_exists?(name)
-      $tables_metadata.hset(key, 'the_geom_type', the_geom_type_value)
-    else
-      self.temporal_the_geom_type = the_geom_type_value
-    end
+    raise CartoDB::InvalidGeomType.new(self.the_geom_type_value) unless CartoDB::VALID_GEOMETRY_TYPES.include?(self.the_geom_type_value)
   end
 
   # if the table is already renamed, we just need to update the name attribute
@@ -1418,19 +1397,6 @@ def update_name_changes
     if @name_changed_from.present? && @name_changed_from != name
       reload
 
-      old_key = Table.key(owner.database_name,"#{owner.database_schema}.#{@name_changed_from}")
-      new_key = key
-
-      begin
-        # update metadata records
-        $tables_metadata.rename(old_key, new_key)
-      rescue StandardError => exception
-        exception_to_raise = CartoDB::BaseCartoDBError.new(
-            "Table update_name_changes(): '#{@name_changed_from}','#{key}' renaming metadata", exception)
-        CartoDB::notify_exception(exception_to_raise, user: owner)
-        errored = true
-      end
-
       unless register_table_only
         begin
           owner.in_database.rename_table(@name_changed_from, name) unless errored
@@ -1455,7 +1421,6 @@ def update_name_changes
       end
 
       if errored
-        $tables_metadata.rename(new_key, old_key)
         raise exception_to_raise
       end
     end

app/models/table/privacy_manager.rb

@@ -47,19 +47,9 @@ def propagate_to(visualization)
       self
     end
 
-    def privacy_for_redis
-      case @table.privacy
-        when ::Table::PRIVACY_PUBLIC, ::Table::PRIVACY_LINK
-          ::Table::PRIVACY_PUBLIC
-        else
-          ::Table::PRIVACY_PRIVATE
-      end
-    end
-
-    def propagate_to_redis_and_varnish
+    def propagate_to_varnish
       raise 'table privacy cannot be nil' unless privacy
       # TODO: Improve this, hack because tiler checks it
-      $tables_metadata.hset redis_key, 'privacy', privacy_for_redis
       invalidate_varnish_cache
       self
     end
@@ -108,9 +98,6 @@ def varnish_key
       end
     end
 
-    def redis_key
-      "rails:#{table.owner.database_name}:#{owner.database_schema}.#{table.name}"
-    end
   end
 end
 

app/models/visualization/member.rb

@@ -703,7 +703,7 @@ def propagate_privacy_to(table)
         if type == TYPE_CANONICAL
           CartoDB::TablePrivacyManager.new(table)
             .set_from(self)
-            .propagate_to_redis_and_varnish
+            .propagate_to_varnish
         end
         self
       end

lib/cartodb/generic_migrator.rb

@@ -16,16 +16,21 @@ def initialize(version)
     end #initialize
 
     def already_migrated?(table)
-      $tables_metadata.hget(table.key, "migrated_to_#{@version}").to_s == "true"
+      $tables_metadata.hget(key(table), "migrated_to_#{@version}").to_s == "true"
     end
 
     def migrated!(table)
       @stats[:tables_migrated] += 1
-      $tables_metadata.hset(table.key, "migrated_to_#{@version}", true)
+      $tables_metadata.hset(key(table), "migrated_to_#{@version}", true)
     end
 
     def log msg
       @logger.debug(msg)
     end
+
+    private
+    def key(table)
+      "rails:#{table.owner.database_name}:#{table.owner.database_schema}.#{table.name}")
+    end
   end
 end

lib/cartodb/migrator20.rb

@@ -55,7 +55,7 @@ def add_table_id(table)
     end
 
     def migrate_table_map(table)
-      map_metadata = JSON.parse($tables_metadata.hget(table.key, 'map_metadata')) rescue {}
+      map_metadata = JSON.parse($tables_metadata.hget(key(table), 'map_metadata')) rescue {}
       map = table.map
 
       # All previous maps were based on google maps
@@ -73,8 +73,8 @@ def migrate_table_map(table)
     end
 
     def migrate_table_layers(table)
-      map_metadata = JSON.parse($tables_metadata.hget(table.key, 'map_metadata')) rescue {}
-      infowindow_metadata = JSON.parse($tables_metadata.hget(table.key, 'infowindow')) rescue {}
+      map_metadata = JSON.parse($tables_metadata.hget(key(table), 'map_metadata')) rescue {}
+      infowindow_metadata = JSON.parse($tables_metadata.hget(key(table), 'infowindow')) rescue {}
 
 
       # Data layer setup
@@ -145,4 +145,10 @@ def migrate_table_layers(table)
       base_layer.save
     end
   end
+
+  private
+  def key(table)
+    "rails:#{table.owner.database_name}:#{table.owner.database_schema}.#{table.name}")
+  end
+
 end

lib/cartodb/migrator21.rb

@@ -51,9 +51,13 @@ def rollback!
       ::Table.db['delete from layers_user_tables']
       # Remove redis keys
       @tables_to_migrate.all.each do |table|
-        $tables_metadata.hdel(table.key, "migrated_to_#{@version}")
+        $tables_metadata.hdel(key(table), "migrated_to_#{@version}")
       end
     end
 
+    private
+    def key(table)
+      "rails:#{table.owner.database_name}:#{table.owner.database_schema}.#{table.name}")
+    end
   end
 end # CartoDB

lib/varnish/lib/cartodb-varnish.rb

@@ -4,7 +4,6 @@ module CartoDB
   class Varnish
     def purge(what)
       ActiveSupport::Notifications.instrument('purge.varnish', what: what) do |payload|
-
         conf = Cartodb::config[:varnish_management]
         if conf['http_port']
           request = Typhoeus::Request.new(