don't send nils to cassandra driver on eventstream insert
During the EventStream refactor we changed the page view inserts from using PageView#changes to PageView#attributes. This was the right thing to do, because #changes won't include columns that have the default value for that column, but cassandra needs to be given any non-nil default values. Not a problem for PageView, but it could be for other models. However, this introduced a performance regression, because #attributes includes nil attributes which means the cassandra driver was sending a DELETE (list of columns) command, creating a new tombstone record in cassandra. The new insert_record() method on the driver removes any nil values before calling update_record. test plan: enable cassandra page views, then visit some pages. after the insert job runs, the page views should have been created properly and still display properly in the history. Change-Id: Iccb967d3ec1f6b295d98b3f330b4b8ffe4508437 Reviewed-on: https://gerrit.instructure.com/20151 Reviewed-by: Jacob Fugal <jacob@instructure.com> Tested-by: Jenkins <jenkins@instructure.com> Product-Review: Brian Palmer <brianp@instructure.com> QA-Review: Brian Palmer <brianp@instructure.com>
This commit is contained in:
parent
76fcade2fb
commit
0c360565a9
|
@ -148,6 +148,14 @@ module Canvas::Cassandra
|
|||
end
|
||||
end
|
||||
|
||||
# same as update_record, but preferred when doing inserts -- it skips
|
||||
# updating columns with nil values, rather than creating tombstone delete
|
||||
# records for them
|
||||
def insert_record(table_name, primary_key_attrs, changes)
|
||||
changes = changes.reject { |k,v| v.is_a?(Array) ? v.last.nil? : v.nil? }
|
||||
update_record(table_name, primary_key_attrs, changes)
|
||||
end
|
||||
|
||||
def select_value(query, *args)
|
||||
result_row = execute(query, *args).fetch
|
||||
result_row && result_row.to_hash.values.first
|
||||
|
|
|
@ -39,7 +39,7 @@ class EventStream
|
|||
|
||||
def insert(record)
|
||||
database.batch do
|
||||
database.update_record(table, { id_column => record.id }, record.attributes)
|
||||
database.insert_record(table, { id_column => record.id }, record.attributes)
|
||||
run_callbacks(:insert, record)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -145,6 +145,18 @@ describe "Canvas::Redis::Cassandra" do
|
|||
end
|
||||
end
|
||||
|
||||
describe "#insert_record" do
|
||||
it "should not delete nils in an AR#attributes style hash" do
|
||||
db.expects(:execute).with("UPDATE test_table SET name = ? WHERE id = ?", "test", 5)
|
||||
db.insert_record("test_table", { :id => 5 }, { :name => "test", :nick => nil })
|
||||
end
|
||||
|
||||
it "should not delete nils in an AR#changes style hash" do
|
||||
db.expects(:execute).with("UPDATE test_table SET name = ? WHERE id = ?", "test", 5)
|
||||
db.insert_record("test_table", { :id => 5 }, { :name => [nil, "test"], :nick => [nil, nil] })
|
||||
end
|
||||
end
|
||||
|
||||
describe "execute and update" do
|
||||
it_should_behave_like "cassandra page views"
|
||||
|
||||
|
|
|
@ -24,6 +24,7 @@ describe EventStream do
|
|||
@database = stub('database')
|
||||
def @database.batch; yield; end
|
||||
def @database.update_record(*args); end
|
||||
def @database.insert_record(*args); end
|
||||
def @database.update(*args); end
|
||||
::Canvas::Cassandra::Database.stubs(:from_config).with(@database_name.to_s).returns(@database)
|
||||
end
|
||||
|
@ -129,24 +130,24 @@ describe EventStream do
|
|||
end
|
||||
|
||||
it "should insert into the configured database" do
|
||||
@database.expects(:update_record).once
|
||||
@database.expects(:insert_record).once
|
||||
@stream.insert(@record)
|
||||
end
|
||||
|
||||
it "should insert into the configured table" do
|
||||
@database.expects(:update_record).with(@table.to_s, anything, anything)
|
||||
@database.expects(:insert_record).with(@table.to_s, anything, anything)
|
||||
@stream.insert(@record)
|
||||
end
|
||||
|
||||
it "should insert by the record's id into the configured id column" do
|
||||
id_column = stub(:to_s => stub('id_column'))
|
||||
@stream.id_column id_column
|
||||
@database.expects(:update_record).with(anything, { id_column.to_s => @record.id }, anything)
|
||||
@database.expects(:insert_record).with(anything, { id_column.to_s => @record.id }, anything)
|
||||
@stream.insert(@record)
|
||||
end
|
||||
|
||||
it "should insert the record's attributes" do
|
||||
@database.expects(:update_record).with(anything, anything, @record.attributes)
|
||||
@database.expects(:insert_record).with(anything, anything, @record.attributes)
|
||||
@stream.insert(@record)
|
||||
end
|
||||
|
||||
|
@ -154,7 +155,7 @@ describe EventStream do
|
|||
spy = stub('spy')
|
||||
@stream.on_insert{ spy.trigger }
|
||||
@database.expects(:batch).once
|
||||
@database.expects(:update_record).never
|
||||
@database.expects(:insert_record).never
|
||||
spy.expects(:trigger).never
|
||||
@stream.insert(@record)
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue