Skip to content

Commit

Permalink
Merge pull request #2362 from twobraids/fix-plugins-integrity-error
Browse files Browse the repository at this point in the history 
fixes Bug 1066316 - make sure plugins_reports_* is properly cleared before replacing a row
  • Loading branch information
@twobraids
twobraids committed Sep 12, 2014
2 parents 1b9caba + 69b9cfa commit 1c03ad5
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 6 deletions.
22 changes: 22 additions & 0 deletions socorro/external/postgresql/crashstorage.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,28 @@ def _save_plugins(self, connection, processed_crash, report_id):
crash_id = processed_crash['uuid']
table_suffix = self._table_suffix_for_crash_id(crash_id)
plugin_reports_table_name = 'plugins_reports_%s' % table_suffix


# why are we deleting first? This might be a reprocessing job and
# the plugins_reports data might already be in the table: a
# straight insert might fail. Why not check to see if there is
# data already there and then just not insert if data is there?
# We may be reprocessing to deal with missing plugin_reports data,
# so just because there is already data there doesn't mean that we
# can skip this. What about using "upsert" sql - that would be fine
# and result in one fewer round trip between client and database,
# but "upsert" sql is opaque and not easy to understand at a
# glance. This was faster to implement. What about using
# "transaction check points"? Too many round trips between the
# client and the server.
plugins_reports_delete_sql = (
'delete from %s where report_id = %%s'
% plugin_reports_table_name
)
execute_no_results(connection,
plugins_reports_delete_sql,
(report_id,))

plugins_reports_insert_sql = (
'insert into %s '
' (report_id, plugin_id, date_processed, version) '
Expand Down
18 changes: 12 additions & 6 deletions socorro/unittest/external/postgresql/test_crashstorage.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -390,16 +390,18 @@ def fetch_all_func(*args):
database = crashstorage.database.return_value = m
m.cursor.return_value.fetchall.side_effect = fetch_all_func
crashstorage.save_processed(a_processed_crash)
eq_(m.cursor.call_count, 6)
eq_(m.cursor.call_count, 7)
eq_(m.cursor().fetchall.call_count, 2)
eq_(m.cursor().execute.call_count, 6)
eq_(m.cursor().execute.call_count, 7)

expected_execute_args = (
(('WITH update_report AS (UPDATE reports_20120402 SET addons_checked=%s, address=%s, app_notes=%s, build=%s, client_crash_date=%s, completed_datetime=%s, cpu_info=%s, cpu_name=%s, date_processed=%s, distributor=%s, distributor_version=%s, email=%s, exploitability=%s, flash_version=%s, hangid=%s, install_age=%s, last_crash=%s, os_name=%s, os_version=%s, processor_notes=%s, process_type=%s, product=%s, productid=%s, reason=%s, release_channel=%s, signature=%s, started_datetime=%s, success=%s, topmost_filenames=%s, truncated=%s, uptime=%s, user_comments=%s, user_id=%s, url=%s, uuid=%s, version=%s WHERE uuid=%s RETURNING id), insert_report AS (INSERT INTO reports_20120402(addons_checked, address, app_notes, build, client_crash_date, completed_datetime, cpu_info, cpu_name, date_processed, distributor, distributor_version, email, exploitability, flash_version, hangid, install_age, last_crash, os_name, os_version, processor_notes, process_type, product, productid, reason, release_channel, signature, started_datetime, success, topmost_filenames, truncated, uptime, user_comments, user_id, url, uuid, version)(SELECT%s as addons_checked, %s as address, %s as app_notes, %s as build, %s as client_crash_date, %s as completed_datetime, %s as cpu_info, %s as cpu_name, %s as date_processed, %s as distributor, %s as distributor_version, %s as email, %s as exploitability, %s as flash_version, %s as hangid, %s as install_age, %s as last_crash, %s as os_name, %s as os_version, %s as processor_notes, %s as process_type, %s as product, %s as productid, %s as reason, %s as release_channel, %s as signature, %s as started_datetime, %s as success, %s as topmost_filenames, %s as truncated, %s as uptime, %s as user_comments, %s as user_id, %s as url, %s as uuid, %s as version WHERE NOT EXISTS(SELECT uuid from reports_20120402 WHERE uuid=%s LIMIT 1)) RETURNING id) SELECT * from update_report UNION ALL SELECT * from insert_report',
[None, '0x1c', '...', '20120309050057', '2012-04-08 10:52:42.0', '2012-04-08 10:56:50.902884', 'None | 0', 'arm', '2012-04-08 10:56:41.558922', None, None, 'bogus@bogus.com', 'high', '[blank]', None, 22385, None, 'Linux', '0.0.0 Linux 2.6.35.7-perf-CL727859 #1 ', 'SignatureTool: signature truncated due to length', 'plugin', 'FennecAndroid', 'FA-888888', 'SIGSEGV', 'default', 'libxul.so@0x117441c', '2012-04-08 10:56:50.440752', True, [], False, 170, None, None, 'http://embarrassing.porn.com', '936ce666-ff3b-4c7a-9674-367fe2120408', '13.0a1', '936ce666-ff3b-4c7a-9674-367fe2120408',
None, '0x1c', '...', '20120309050057', '2012-04-08 10:52:42.0', '2012-04-08 10:56:50.902884', 'None | 0', 'arm', '2012-04-08 10:56:41.558922', None, None, 'bogus@bogus.com', 'high', '[blank]', None, 22385, None, 'Linux', '0.0.0 Linux 2.6.35.7-perf-CL727859 #1 ', 'SignatureTool: signature truncated due to length', 'plugin', 'FennecAndroid', 'FA-888888', 'SIGSEGV', 'default', 'libxul.so@0x117441c', '2012-04-08 10:56:50.440752', True, [], False, 170, None, None, 'http://embarrassing.porn.com', '936ce666-ff3b-4c7a-9674-367fe2120408', '13.0a1', '936ce666-ff3b-4c7a-9674-367fe2120408']),),
(('select id from plugins where filename = %s and name = %s',
('dwight.txt', 'wilma')),),
(('delete from plugins_reports_20120402 where report_id = %s',
(666, )),),
(('insert into plugins_reports_20120402 (report_id, plugin_id, date_processed, version) values (%s, %s, %s, %s)',
(666, 23, '2012-04-08 10:56:41.558922', '69')),),
(('delete from extensions_20120402 where report_id = %s',
Expand Down Expand Up @@ -455,9 +457,9 @@ def fetch_all_func(*args):
database = crashstorage.database.return_value = m
m.cursor.return_value.fetchall.side_effect = fetch_all_func
crashstorage.save_processed(a_processed_crash)
eq_(m.cursor.call_count, 7)
eq_(m.cursor.call_count, 8)
eq_(m.cursor().fetchall.call_count, 3)
eq_(m.cursor().execute.call_count, 7)
eq_(m.cursor().execute.call_count, 8)

expected_execute_args = (
(('WITH update_report AS (UPDATE reports_20120402 SET addons_checked=%s, address=%s, app_notes=%s, build=%s, client_crash_date=%s, completed_datetime=%s, cpu_info=%s, cpu_name=%s, date_processed=%s, distributor=%s, distributor_version=%s, email=%s, exploitability=%s, flash_version=%s, hangid=%s, install_age=%s, last_crash=%s, os_name=%s, os_version=%s, processor_notes=%s, process_type=%s, product=%s, productid=%s, reason=%s, release_channel=%s, signature=%s, started_datetime=%s, success=%s, topmost_filenames=%s, truncated=%s, uptime=%s, user_comments=%s, user_id=%s, url=%s, uuid=%s, version=%s WHERE uuid=%s RETURNING id), insert_report AS (INSERT INTO reports_20120402(addons_checked, address, app_notes, build, client_crash_date, completed_datetime, cpu_info, cpu_name, date_processed, distributor, distributor_version, email, exploitability, flash_version, hangid, install_age, last_crash, os_name, os_version, processor_notes, process_type, product, productid, reason, release_channel, signature, started_datetime, success, topmost_filenames, truncated, uptime, user_comments, user_id, url, uuid, version)(SELECT%s as addons_checked, %s as address, %s as app_notes, %s as build, %s as client_crash_date, %s as completed_datetime, %s as cpu_info, %s as cpu_name, %s as date_processed, %s as distributor, %s as distributor_version, %s as email, %s as exploitability, %s as flash_version, %s as hangid, %s as install_age, %s as last_crash, %s as os_name, %s as os_version, %s as processor_notes, %s as process_type, %s as product, %s as productid, %s as reason, %s as release_channel, %s as signature, %s as started_datetime, %s as success, %s as topmost_filenames, %s as truncated, %s as uptime, %s as user_comments, %s as user_id, %s as url, %s as uuid, %s as version WHERE NOT EXISTS(SELECT uuid from reports_20120402 WHERE uuid=%s LIMIT 1)) RETURNING id) SELECT * from update_report UNION ALL SELECT * from insert_report',
Expand All @@ -467,6 +469,8 @@ def fetch_all_func(*args):
('dwight.txt', 'wilma')),),
(('insert into plugins (filename, name) values (%s, %s) returning id',
('dwight.txt', 'wilma')),),
(('delete from plugins_reports_20120402 where report_id = %s',
(666, )),),
(('insert into plugins_reports_20120402 (report_id, plugin_id, date_processed, version) values (%s, %s, %s, %s)',
(666, 23, '2012-04-08 10:56:41.558922', '69')),),
(('delete from extensions_20120402 where report_id = %s',
Expand Down Expand Up @@ -582,9 +586,9 @@ def broken_connection(*args):
database = crashstorage.database.return_value = m
m.cursor.side_effect = broken_connection
crashstorage.save_processed(a_processed_crash)
eq_(m.cursor.call_count, 9)
eq_(m.cursor.call_count, 10)
eq_(m.cursor().fetchall.call_count, 3)
eq_(m.cursor().execute.call_count, 7)
eq_(m.cursor().execute.call_count, 8)

expected_execute_args = (
(('WITH update_report AS (UPDATE reports_20120402 SET addons_checked=%s, address=%s, app_notes=%s, build=%s, client_crash_date=%s, completed_datetime=%s, cpu_info=%s, cpu_name=%s, date_processed=%s, distributor=%s, distributor_version=%s, email=%s, exploitability=%s, flash_version=%s, hangid=%s, install_age=%s, last_crash=%s, os_name=%s, os_version=%s, processor_notes=%s, process_type=%s, product=%s, productid=%s, reason=%s, release_channel=%s, signature=%s, started_datetime=%s, success=%s, topmost_filenames=%s, truncated=%s, uptime=%s, user_comments=%s, user_id=%s, url=%s, uuid=%s, version=%s WHERE uuid=%s RETURNING id), insert_report AS (INSERT INTO reports_20120402(addons_checked, address, app_notes, build, client_crash_date, completed_datetime, cpu_info, cpu_name, date_processed, distributor, distributor_version, email, exploitability, flash_version, hangid, install_age, last_crash, os_name, os_version, processor_notes, process_type, product, productid, reason, release_channel, signature, started_datetime, success, topmost_filenames, truncated, uptime, user_comments, user_id, url, uuid, version)(SELECT%s as addons_checked, %s as address, %s as app_notes, %s as build, %s as client_crash_date, %s as completed_datetime, %s as cpu_info, %s as cpu_name, %s as date_processed, %s as distributor, %s as distributor_version, %s as email, %s as exploitability, %s as flash_version, %s as hangid, %s as install_age, %s as last_crash, %s as os_name, %s as os_version, %s as processor_notes, %s as process_type, %s as product, %s as productid, %s as reason, %s as release_channel, %s as signature, %s as started_datetime, %s as success, %s as topmost_filenames, %s as truncated, %s as uptime, %s as user_comments, %s as user_id, %s as url, %s as uuid, %s as version WHERE NOT EXISTS(SELECT uuid from reports_20120402 WHERE uuid=%s LIMIT 1)) RETURNING id) SELECT * from update_report UNION ALL SELECT * from insert_report',
Expand All @@ -594,6 +598,8 @@ def broken_connection(*args):
('dwight.txt', 'wilma')),),
(('insert into plugins (filename, name) values (%s, %s) returning id',
('dwight.txt', 'wilma')),),
(('delete from plugins_reports_20120402 where report_id = %s',
(666, )),),
(('insert into plugins_reports_20120402 (report_id, plugin_id, date_processed, version) values (%s, %s, %s, %s)',
(666, 23, '2012-04-08 10:56:41.558922', '69')),),
(('delete from extensions_20120402 where report_id = %s',
Expand Down

0 comments on commit 1c03ad5

Please sign in to comment.