Skip to content

Commit

Permalink
Fixes based on feedback r=twobraids
Browse files Browse the repository at this point in the history 
* Change RETURNING to 1 or 2 based on update or insert
* Remove savepoint and try/except block for processed_crashes insert
* Fix error in UPSERT SQL
  • Loading branch information
@selenamarie
selenamarie committed Feb 26, 2014
1 parent 5d3f80e commit 5be75a5
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 39 deletions.
39 changes: 15 additions & 24 deletions socorro/external/postgresql/crashstorage.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,48 +186,39 @@ def _save_processed_crash(self, connection, processed_crash):
)
upsert_sql = """
WITH
update_processed_crash (
update_processed_crash AS (
UPDATE %(table)s SET
processed_crash = %%(processed_json)s,
date_processed = %%(date_processed)s
WHERE uuid = %%(uuid)s
RETURNING '%(uuid)s'
RETURNING 1
),
insert_processed_crash (
insert_processed_crash AS (
INSERT INTO %(table)s (uuid, processed_crash, date_processed)
VALUES (%%(uuid)s, %%(processed_json)s, %%(date_processed)s)
WHERE NOT EXISTS (
SELECT uuid from %(table)s
WHERE
uuid = %%(uuid)s
LIMIT 1
( SELECT
%%(uuid)s as uuid,
%%(processed_json)s as processed_crash,
%%(date_processed)s as date_processed
WHERE NOT EXISTS (
SELECT uuid from %(table)s
WHERE
uuid = %%(uuid)s
LIMIT 1
)
)
RETURNING '%(uuid)s'
RETURNING 2
)
SELECT * from update_processed_crash
UNION ALL
SELECT * from insert_processed_crash
""" % {'table': processed_crashes_table_name, 'uuid': crash_id}

savepoint_name = threading.currentThread().getName().replace('-', '')
values = {
'processed_json': json.dumps(processed_crash, cls=JsonDTEncoder),
'date_processed': processed_crash["date_processed"],
'uuid': crash_id
}
execute_no_results(connection, "savepoint %s" % savepoint_name)
try:
execute_no_results(connection, upsert_sql, values)
execute_no_results(
connection,
"release savepoint %s" % savepoint_name
)
except self.config.database_class.Error:
# Rollback on any database Error; postgres logs this so we won't.
execute_no_results(
connection,
"rollback to savepoint %s" % savepoint_name
)
execute_no_results(connection, upsert_sql, values)

#--------------------------------------------------------------------------
def _save_processed_report(self, connection, processed_crash):
Expand Down
24 changes: 9 additions & 15 deletions socorro/unittest/external/postgresql/test_crashstorage.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,9 +366,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)
self.assertEqual(m.cursor.call_count, 9)
self.assertEqual(m.cursor.call_count, 7)
self.assertEqual(m.cursor().fetchall.call_count, 2)
self.assertEqual(m.cursor().execute.call_count, 9)
self.assertEqual(m.cursor().execute.call_count, 7)

expected_execute_args = (
(('savepoint MainThread', None),),
Expand All @@ -382,11 +382,9 @@ def fetch_all_func(*args):
(666, 23, '2012-04-08 10:56:41.558922', '69')),),
(('insert into extensions_20120402 (report_id, date_processed, extension_key, extension_id, extension_version)values (%s, %s, %s, %s, %s)',
(666, '2012-04-08 10:56:41.558922', 0, '{1a5dabbd-0e74-41da-b532-a364bb552cab}', '1.0.4.1')),),
(('savepoint MainThread', None),),
(("""WITH update_processed_crash ( UPDATE processed_crashes_20120402 SET processed_crash = %(processed_json)s, date_processed = %(date_processed)s WHERE uuid = %(uuid)s RETURNING \'936ce666-ff3b-4c7a-9674-367fe2120408\'), insert_processed_crash ( INSERT INTO processed_crashes_20120402 (uuid, processed_crash, date_processed) VALUES (%(uuid)s, %(processed_json)s, %(date_processed)s) WHERE NOT EXISTS ( SELECT uuid from processed_crashes_20120402 WHERE uuid = %(uuid)s LIMIT 1) RETURNING \'936ce666-ff3b-4c7a-9674-367fe2120408\') SELECT * from update_processed_crash UNION ALL SELECT * from insert_processed_crash """,
(("""WITH update_processed_crash AS ( UPDATE processed_crashes_20120402 SET processed_crash = %(processed_json)s, date_processed = %(date_processed)s WHERE uuid = %(uuid)s RETURNING 1), insert_processed_crash AS ( INSERT INTO processed_crashes_20120402 (uuid, processed_crash, date_processed) ( SELECT %(uuid)s as uuid, %(processed_json)s as processed_crash, %(date_processed)s as date_processed WHERE NOT EXISTS ( SELECT uuid from processed_crashes_20120402 WHERE uuid = %(uuid)s LIMIT 1)) RETURNING 2) SELECT * from update_processed_crash UNION ALL SELECT * from insert_processed_crash """,
{'uuid': '936ce666-ff3b-4c7a-9674-367fe2120408', 'processed_json': '{"startedDateTime": "2012-04-08 10:56:50.440752", "crashedThread": 8, "cpu_info": "None | 0", "PluginName": "wilma", "install_age": 22385, "topmost_filenames": [], "user_comments": null, "user_id": null, "uuid": "936ce666-ff3b-4c7a-9674-367fe2120408", "flash_version": "[blank]", "os_version": "0.0.0 Linux 2.6.35.7-perf-CL727859 #1 ", "PluginVersion": "69", "addons_checked": null, "completeddatetime": "2012-04-08 10:56:50.902884", "productid": "FA-888888", "success": true, "exploitability": "high", "client_crash_date": "2012-04-08 10:52:42.0", "PluginFilename": "dwight.txt", "dump": "...", "truncated": false, "product": "FennecAndroid", "distributor": null, "processor_notes": "SignatureTool: signature truncated due to length", "uptime": 170, "release_channel": "default", "distributor_version": null, "process_type": "plugin", "id": 361399767, "hangid": null, "version": "13.0a1", "build": "20120309050057", "ReleaseChannel": "default", "email": "bogus@bogus.com", "app_notes": "...", "os_name": "Linux", "last_crash": null, "date_processed": "2012-04-08 10:56:41.558922", "cpu_name": "arm", "reason": "SIGSEGV", "address": "0x1c", "url": "http://embarrassing.porn.com", "signature": "libxul.so@0x117441c", "addons": [["{1a5dabbd-0e74-41da-b532-a364bb552cab}", "1.0.4.1"]]}', 'date_processed': '2012-04-08 10:56:41.558922'}
),),
(('release savepoint MainThread', None),),
)

actual_execute_args = m.cursor().execute.call_args_list
Expand Down Expand Up @@ -434,9 +432,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)
self.assertEqual(m.cursor.call_count, 10)
self.assertEqual(m.cursor.call_count, 8)
self.assertEqual(m.cursor().fetchall.call_count, 3)
self.assertEqual(m.cursor().execute.call_count, 10)
self.assertEqual(m.cursor().execute.call_count, 8)

expected_execute_args = (
(('savepoint MainThread', None),),
Expand All @@ -451,11 +449,9 @@ def fetch_all_func(*args):
(666, 23, '2012-04-08 10:56:41.558922', '69')),),
(('insert into extensions_20120402 (report_id, date_processed, extension_key, extension_id, extension_version)values (%s, %s, %s, %s, %s)',
(666, '2012-04-08 10:56:41.558922', 0, '{1a5dabbd-0e74-41da-b532-a364bb552cab}', '1.0.4.1')),),
(('savepoint MainThread', None),),
(("""WITH update_processed_crash ( UPDATE processed_crashes_20120402 SET processed_crash = %(processed_json)s, date_processed = %(date_processed)s WHERE uuid = %(uuid)s RETURNING \'936ce666-ff3b-4c7a-9674-367fe2120408\'), insert_processed_crash ( INSERT INTO processed_crashes_20120402 (uuid, processed_crash, date_processed) VALUES (%(uuid)s, %(processed_json)s, %(date_processed)s) WHERE NOT EXISTS ( SELECT uuid from processed_crashes_20120402 WHERE uuid = %(uuid)s LIMIT 1) RETURNING \'936ce666-ff3b-4c7a-9674-367fe2120408\') SELECT * from update_processed_crash UNION ALL SELECT * from insert_processed_crash """,
(("""WITH update_processed_crash AS ( UPDATE processed_crashes_20120402 SET processed_crash = %(processed_json)s, date_processed = %(date_processed)s WHERE uuid = %(uuid)s RETURNING 1), insert_processed_crash AS ( INSERT INTO processed_crashes_20120402 (uuid, processed_crash, date_processed) ( SELECT %(uuid)s as uuid, %(processed_json)s as processed_crash, %(date_processed)s as date_processed WHERE NOT EXISTS ( SELECT uuid from processed_crashes_20120402 WHERE uuid = %(uuid)s LIMIT 1)) RETURNING 2) SELECT * from update_processed_crash UNION ALL SELECT * from insert_processed_crash """,
{'uuid': '936ce666-ff3b-4c7a-9674-367fe2120408', 'processed_json': '{"startedDateTime": "2012-04-08 10:56:50.440752", "crashedThread": 8, "cpu_info": "None | 0", "PluginName": "wilma", "install_age": 22385, "topmost_filenames": [], "user_comments": null, "user_id": null, "uuid": "936ce666-ff3b-4c7a-9674-367fe2120408", "flash_version": "[blank]", "os_version": "0.0.0 Linux 2.6.35.7-perf-CL727859 #1 ", "PluginVersion": "69", "addons_checked": null, "completeddatetime": "2012-04-08 10:56:50.902884", "productid": "FA-888888", "success": true, "exploitability": "high", "client_crash_date": "2012-04-08 10:52:42.0", "PluginFilename": "dwight.txt", "dump": "...", "truncated": false, "product": "FennecAndroid", "distributor": null, "processor_notes": "SignatureTool: signature truncated due to length", "uptime": 170, "release_channel": "default", "distributor_version": null, "process_type": "plugin", "id": 361399767, "hangid": null, "version": "13.0a1", "build": "20120309050057", "ReleaseChannel": "default", "email": "bogus@bogus.com", "app_notes": "...", "os_name": "Linux", "last_crash": null, "date_processed": "2012-04-08 10:56:41.558922", "cpu_name": "arm", "reason": "SIGSEGV", "address": "0x1c", "url": "http://embarrassing.porn.com", "signature": "libxul.so@0x117441c", "addons": [["{1a5dabbd-0e74-41da-b532-a364bb552cab}", "1.0.4.1"]]}', 'date_processed': '2012-04-08 10:56:41.558922'}
),),
(('release savepoint MainThread', None),),
)

actual_execute_args = m.cursor().execute.call_args_list
Expand Down Expand Up @@ -561,9 +557,9 @@ def broken_connection(*args):
database = crashstorage.database.return_value = m
m.cursor.side_effect = broken_connection
crashstorage.save_processed(a_processed_crash)
self.assertEqual(m.cursor.call_count, 12)
self.assertEqual(m.cursor.call_count, 10)
self.assertEqual(m.cursor().fetchall.call_count, 3)
self.assertEqual(m.cursor().execute.call_count, 10)
self.assertEqual(m.cursor().execute.call_count, 8)

expected_execute_args = (
(('savepoint MainThread', None),),
Expand All @@ -578,11 +574,9 @@ def broken_connection(*args):
(666, 23, '2012-04-08 10:56:41.558922', '69')),),
(('insert into extensions_20120402 (report_id, date_processed, extension_key, extension_id, extension_version)values (%s, %s, %s, %s, %s)',
(666, '2012-04-08 10:56:41.558922', 0, '{1a5dabbd-0e74-41da-b532-a364bb552cab}', '1.0.4.1')),),
(('savepoint MainThread', None),),
(("""WITH update_processed_crash ( UPDATE processed_crashes_20120402 SET processed_crash = %(processed_json)s, date_processed = %(date_processed)s WHERE uuid = %(uuid)s RETURNING \'936ce666-ff3b-4c7a-9674-367fe2120408\'), insert_processed_crash ( INSERT INTO processed_crashes_20120402 (uuid, processed_crash, date_processed) VALUES (%(uuid)s, %(processed_json)s, %(date_processed)s) WHERE NOT EXISTS ( SELECT uuid from processed_crashes_20120402 WHERE uuid = %(uuid)s LIMIT 1) RETURNING \'936ce666-ff3b-4c7a-9674-367fe2120408\') SELECT * from update_processed_crash UNION ALL SELECT * from insert_processed_crash """,
(("""WITH update_processed_crash AS ( UPDATE processed_crashes_20120402 SET processed_crash = %(processed_json)s, date_processed = %(date_processed)s WHERE uuid = %(uuid)s RETURNING 1), insert_processed_crash AS ( INSERT INTO processed_crashes_20120402 (uuid, processed_crash, date_processed) ( SELECT %(uuid)s as uuid, %(processed_json)s as processed_crash, %(date_processed)s as date_processed WHERE NOT EXISTS ( SELECT uuid from processed_crashes_20120402 WHERE uuid = %(uuid)s LIMIT 1)) RETURNING 2) SELECT * from update_processed_crash UNION ALL SELECT * from insert_processed_crash """,
{'uuid': '936ce666-ff3b-4c7a-9674-367fe2120408', 'processed_json': '{"startedDateTime": "2012-04-08 10:56:50.440752", "crashedThread": 8, "cpu_info": "None | 0", "PluginName": "wilma", "install_age": 22385, "topmost_filenames": [], "user_comments": null, "user_id": null, "uuid": "936ce666-ff3b-4c7a-9674-367fe2120408", "flash_version": "[blank]", "os_version": "0.0.0 Linux 2.6.35.7-perf-CL727859 #1 ", "PluginVersion": "69", "addons_checked": null, "completeddatetime": "2012-04-08 10:56:50.902884", "productid": "FA-888888", "success": true, "exploitability": "high", "client_crash_date": "2012-04-08 10:52:42.0", "PluginFilename": "dwight.txt", "dump": "...", "truncated": false, "product": "FennecAndroid", "distributor": null, "processor_notes": "SignatureTool: signature truncated due to length", "uptime": 170, "release_channel": "default", "distributor_version": null, "process_type": "plugin", "id": 361399767, "hangid": null, "version": "13.0a1", "build": "20120309050057", "ReleaseChannel": "default", "email": "bogus@bogus.com", "app_notes": "...", "os_name": "Linux", "last_crash": null, "date_processed": "2012-04-08 10:56:41.558922", "cpu_name": "arm", "reason": "SIGSEGV", "address": "0x1c", "url": "http://embarrassing.porn.com", "signature": "libxul.so@0x117441c", "addons": [["{1a5dabbd-0e74-41da-b532-a364bb552cab}", "1.0.4.1"]]}', 'date_processed': '2012-04-08 10:56:41.558922'}
),),
(('release savepoint MainThread', None),),
)

actual_execute_args = m.cursor().execute.call_args_list
Expand Down

0 comments on commit 5be75a5

Please sign in to comment.