Skip to content

Commit

Permalink
WIP compare decimal quota_* columns correctly in batch saver
Browse files Browse the repository at this point in the history
  • Loading branch information
cben committed Feb 12, 2018
1 parent 3e0195f commit 19c05a2
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 7 deletions.
1 change: 1 addition & 0 deletions app/models/ems_refresh/save_inventory_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def fetch(hash)

# Coerce each hash value into the db column type for valid lookup during fetch_path
coerced_hash_values = hash_values.zip(key_attribute_types).collect do |value, type|
raise "BOO" if value.hash != type.cast(value).hash
type.cast(value)
end

Expand Down
12 changes: 7 additions & 5 deletions app/models/manager_refresh/save_collection/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ def save_inventory_object_inventory(ems, inventory_collection)
_log.debug("Saving collection #{inventory_collection} of size #{inventory_collection.size} to"\
" the database, for the manager: '#{ems.name}'...")

if inventory_collection.custom_save_block.present?
_log.debug("Saving collection #{inventory_collection} using a custom save block")
inventory_collection.custom_save_block.call(ems, inventory_collection)
else
save_inventory(inventory_collection)
Benchmark.realtime_block("SaveCollection #{inventory_collection.name}") do
if inventory_collection.custom_save_block.present?
_log.debug("Saving collection #{inventory_collection} using a custom save block")
inventory_collection.custom_save_block.call(ems, inventory_collection)
else
save_inventory(inventory_collection)
end
end
_log.debug("Saving collection #{inventory_collection}, for the manager: '#{ems.name}'...Complete")
inventory_collection.saved = true
Expand Down
2 changes: 2 additions & 0 deletions app/models/manager_refresh/save_collection/saver/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def initialize(inventory_collection)
@arel_primary_key = @model_class.arel_attribute(@primary_key)
@unique_index_keys = inventory_collection.manager_ref_to_cols.map(&:to_sym)
@unique_index_keys_to_s = inventory_collection.manager_ref_to_cols.map(&:to_s)
@unique_index_types = @unique_index_keys_to_s.map { |a| [a, @model_class.type_for_attribute(a)] }.to_h
@select_keys = [@primary_key] + @unique_index_keys_to_s
@unique_db_primary_keys = Set.new
@unique_db_indexes = Set.new
Expand Down Expand Up @@ -82,6 +83,7 @@ def save!(association)
attributes_index = {}
inventory_objects_index = {}
inventory_collection.each do |inventory_object|
# TODO(cben): quota columns cast?
attributes = inventory_object.attributes(inventory_collection)
index = build_stringified_reference(attributes, unique_index_keys)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,12 @@ def save!(association)
all_attribute_keys = Set.new + inventory_collection.batch_extra_attributes

inventory_collection.each do |inventory_object|
# TODO(cben): quota columns cast?
attributes = inventory_object.attributes_with_keys(inventory_collection, all_attribute_keys)
index = build_stringified_reference(attributes, unique_index_keys)
puts "A #{all_attribute_keys} #{attributes.collect(&:class)}" if inventory_collection.name == :container_quota_items
byebug if inventory_collection.name == :container_quota_items && !$A
p index if index.match? /\.(0000|9999)/

# Interesting fact: not building attributes_index and using only inventory_objects_index doesn't do much
# of a difference, since the most objects inside are shared.
Expand Down Expand Up @@ -100,22 +104,33 @@ def update_or_destroy_records!(records_batch_iterator, inventory_objects_index,

next unless assert_distinct_relation(primary_key_value)

# Incoming values are in SQL string form.
# TODO(lsmola) unify this behavior with object_index_with_keys method in InventoryCollection
# TODO(lsmola) maybe we can drop the whole pure sql fetching, since everything will be targeted refresh
# with streaming refresh? Maybe just metrics and events will not be, but those should be upsert only
index = unique_index_keys_to_s.map do |attribute|
value = record_key(record, attribute)
if attribute == "timestamp"
type = model_class.type_for_attribute(attribute)
type.cast(record_key(record, attribute)).utc.iso8601.to_s
type.cast(value).utc.iso8601.to_s
elsif %w(quota_desired quota_enforced quota_observed).include?(attribute)
# Postgres formats decimal columns with fixed number of digits e.g. '0.100'.
# Need to parse and let Ruby format the value to have a comparable string.
type = model_class.type_for_attribute(attribute)
type.cast(value).to_s
else
record_key(record, attribute).to_s
value.to_s
end
end.join("__")

inventory_object = inventory_objects_index.delete(index)
hash = attributes_index.delete(index)

if inventory_object.nil?
parent_quota = ContainerQuota.find(record_key(record, 'container_quota_id')) rescue nil
parent_proj = ContainerProject.find(parent_quota.try(:container_project_id) || record_key(record, 'container_project_id')) rescue nil
puts("/ proj: #{parent_proj.try(:name)} / quota: #{parent_quota.try(:name)} / " +
"[#{inventory_collection.name}] #{index.inspect} from DB not among inv objs: #{inventory_objects_index.keys}")
# Record was found in the DB but not sent for saving, that means it doesn't exist anymore and we should
# delete it from the DB.
if inventory_collection.delete_allowed?
Expand Down

0 comments on commit 19c05a2

Please sign in to comment.