From 81d9e44b71f9ed6ec607cb0eb966265a2c524b90 Mon Sep 17 00:00:00 2001 From: Jana Sloukova Date: Tue, 14 Mar 2017 17:15:26 +0100 Subject: [PATCH 1/2] Setting of datastore_active flag moved to separate function --- ckanext/datastore/logic/action.py | 95 ++++++++++++++++--------------- 1 file changed, 50 insertions(+), 45 deletions(-) diff --git a/ckanext/datastore/logic/action.py b/ckanext/datastore/logic/action.py index e519670bf19..8d45f837b91 100644 --- a/ckanext/datastore/logic/action.py +++ b/ckanext/datastore/logic/action.py @@ -149,47 +149,7 @@ def datastore_create(context, data_dict): log.debug( 'Setting datastore_active=True on resource {0}'.format(resource.id) ) - # issue #3245: race condition - update_dict = {'datastore_active': True} - - # get extras(for entity update) and package_id(for search index update) - res_query = model.Session.query( - model.resource_table.c.extras, - model.resource_table.c.package_id - ).filter( - model.Resource.id == data_dict['resource_id'] - ) - extras, package_id = res_query.one() - - # update extras in database for record and its revision - extras.update(update_dict) - res_query.update({'extras': extras}, synchronize_session=False) - - model.Session.query(model.resource_revision_table).filter( - model.ResourceRevision.id == data_dict['resource_id'], - model.ResourceRevision.current is True - ).update({'extras': extras}, synchronize_session=False) - - model.Session.commit() - - # get package with updated resource from solr - # find changed resource, patch it and reindex package - psi = search.PackageSearchIndex() - solr_query = search.PackageSearchQuery() - q = { - 'q': 'id:"{0}"'.format(package_id), - 'fl': 'data_dict', - 'wt': 'json', - 'fq': 'site_id:"%s"' % config.get('ckan.site_id'), - 'rows': 1 - } - for record in solr_query.run(q)['results']: - solr_data_dict = json.loads(record['data_dict']) - for resource in solr_data_dict['resources']: - if resource['id'] == data_dict['resource_id']: - resource.update(update_dict) - psi.index_package(solr_data_dict) - break + set_datastore_active_flag(model, data_dict, True) result.pop('id', None) result.pop('private', None) @@ -396,11 +356,9 @@ def datastore_delete(context, data_dict): if (not data_dict.get('filters') and resource.extras.get('datastore_active') is True): log.debug( - 'Setting datastore_active=True on resource {0}'.format(resource.id) + 'Setting datastore_active=False on resource {0}'.format(resource.id) ) - p.toolkit.get_action('resource_patch')( - context, {'id': data_dict['resource_id'], - 'datastore_active': False}) + set_datastore_active_flag(model, data_dict, False) result.pop('id', None) result.pop('connection_url') @@ -598,6 +556,53 @@ def datastore_make_public(context, data_dict): db.make_public(context, data_dict) +def set_datastore_active_flag(model, data_dict, flag): + ''' + Set appropriate datastore_active flag on CKAN resource. + + Called after creation or deletion of DataStore table. + ''' + update_dict = {'datastore_active': flag} + + # get extras(for entity update) and package_id(for search index update) + res_query = model.Session.query( + model.resource_table.c.extras, + model.resource_table.c.package_id + ).filter( + model.Resource.id == data_dict['resource_id'] + ) + extras, package_id = res_query.one() + + # update extras in database for record and its revision + extras.update(update_dict) + res_query.update({'extras': extras}, synchronize_session=False) + model.Session.query(model.resource_revision_table).filter( + model.ResourceRevision.id == data_dict['resource_id'], + model.ResourceRevision.current is True + ).update({'extras': extras}, synchronize_session=False) + + model.Session.commit() + + # get package with updated resource from solr + # find changed resource, patch it and reindex package + psi = search.PackageSearchIndex() + solr_query = search.PackageSearchQuery() + q = { + 'q': 'id:"{0}"'.format(package_id), + 'fl': 'data_dict', + 'wt': 'json', + 'fq': 'site_id:"%s"' % config.get('ckan.site_id'), + 'rows': 1 + } + for record in solr_query.run(q)['results']: + solr_data_dict = json.loads(record['data_dict']) + for resource in solr_data_dict['resources']: + if resource['id'] == data_dict['resource_id']: + resource.update(update_dict) + psi.index_package(solr_data_dict) + break + + def _resource_exists(context, data_dict): ''' Returns true if the resource exists in CKAN and in the datastore ''' model = _get_or_bust(context, 'model') From 0e35ecf1e267c62faea85d46b6190a281a378557 Mon Sep 17 00:00:00 2001 From: Ian Ward Date: Thu, 16 Mar 2017 10:39:08 -0400 Subject: [PATCH 2/2] [#3481] comment explaining approach --- ckanext/datastore/logic/action.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ckanext/datastore/logic/action.py b/ckanext/datastore/logic/action.py index 8d45f837b91..27e74599668 100644 --- a/ckanext/datastore/logic/action.py +++ b/ckanext/datastore/logic/action.py @@ -562,6 +562,9 @@ def set_datastore_active_flag(model, data_dict, flag): Called after creation or deletion of DataStore table. ''' + # We're modifying the resource extra directly here to avoid a + # race condition, see issue #3245 for details and plan for a + # better fix update_dict = {'datastore_active': flag} # get extras(for entity update) and package_id(for search index update)