From d847dfb3e0ab7eaa13d697fb380902451ca2b850 Mon Sep 17 00:00:00 2001 From: willtp87 Date: Tue, 1 Dec 2015 14:21:27 -0400 Subject: [PATCH 1/4] Optionaly using semaphores on APIM. --- includes/admin.form.inc | 18 ++++++++ includes/tuque_wrapper.inc | 88 ++++++++++++++++++++++++++++++++++++-- islandora.install | 2 + 3 files changed, 105 insertions(+), 3 deletions(-) diff --git a/includes/admin.form.inc b/includes/admin.form.inc index 9f64cb6d..c233be2c 100644 --- a/includes/admin.form.inc +++ b/includes/admin.form.inc @@ -61,6 +61,24 @@ function islandora_repository_admin(array $form, array &$form_state) { '#description' => t('HTTP caching can reduce network traffic, by allowing clients to used cached copies.'), '#default_value' => variable_get('islandora_use_datastream_cache_headers', TRUE), ), + 'islandora_use_object_semaphores' => array( + '#type' => 'checkbox', + '#title' => t('Make Processes Claim Objects for Modification'), + '#description' => t('Enabling this will increase stability of Fedora at high concurrency but will incur a heavy performance hit.'), + '#default_value' => variable_get('islandora_use_object_semaphores', FALSE), + ), + 'islandora_semaphore_period' => array( + '#type' => 'textfield', + '#title' => t('Time to Claim Objects for'), + '#default_value' => variable_get('islandora_semaphore_period', 600), + '#description' => t('Time in seconds to claim objects for modification.'), + '#required' => TRUE, + '#states' => array( + 'invisible' => array( + ':input[name="islandora_use_object_semaphores"]' => array('checked' => FALSE), + ), + ), + ), 'islandora_defer_derivatives_on_ingest' => array( '#type' => 'checkbox', '#title' => t('Defer derivative generation during ingest'), diff --git a/includes/tuque_wrapper.inc b/includes/tuque_wrapper.inc index e7ee75c2..5f09985b 100644 --- a/includes/tuque_wrapper.inc +++ b/includes/tuque_wrapper.inc @@ -371,7 +371,7 @@ class IslandoraFedoraApiM extends FedoraApiM { if ($context['block']) { throw new Exception('Modify Datastream was blocked.'); } - return parent::modifyDatastream($pid, $dsid, $params); + return $this->callParentWithLocking('modifyDatastream', $pid, $pid, $dsid, $params); } /** @@ -391,7 +391,7 @@ class IslandoraFedoraApiM extends FedoraApiM { if ($context['block']) { throw new Exception('Modify Object was blocked.'); } - return parent::modifyObject($pid, $params); + return $this->callParentWithLocking('modifyObject', $pid, $pid, $params); } /** @@ -422,7 +422,7 @@ class IslandoraFedoraApiM extends FedoraApiM { return ''; default: - $ret = parent::purgeObject($pid, $log_message); + $ret = $this->callParentWithLocking('purgeObject', $pid, $pid, $log_message); islandora_invoke_object_hooks(ISLANDORA_OBJECT_PURGED_HOOK, $models, $pid); return $ret; } @@ -436,6 +436,88 @@ class IslandoraFedoraApiM extends FedoraApiM { } } + /** + * Wraps purgeDatastream for semaphore locking. + * + * @see FedoraApiM::purgeDatastream + */ + public function purgeDatastream($pid, $dsid, $params = array()) { + return $this->callParentWithLocking('purgeDatastream', $pid, $pid, $dsid, $params); + } + + /** + * Wraps ingest for semaphore locking. + * + * @see FedoraApiM::ingest + */ + public function ingest($params = array()) { + if (isset($params['pid'])) { + return $this->callParentWithLocking('ingest', $params['pid'], $params); + } + else { + return parent::ingest($params); + } + } + + /** + * Wraps addDatastream for semaphore locking. + * + * @see FedoraApiM::addDatastream + */ + public function addDatastream($pid, $dsid, $type, $file, $params) { + return $this->callParentWithLocking('addDatastream', $pid, $pid, $dsid, $type, $file, $params); + } + + /** + * Wraps addRelationship for semaphore locking. + * + * @see FedoraApiM::addRelationship + */ + public function addRelationship($pid, $relationship, $is_literal, $datatype = NULL) { + return $this->callParentWithLocking('addRelationship', $pid, $pid, $relationship, $is_literal, $datatype); + } + + /** + * Call a parent function while using semaphores as configured. + * + * All extra arguments are passed along to the callback. + * + * @param callable $callback + * The method we are wrapping. + * @param string $pid + * The PID to create a semaphore for. + */ + protected function callParentWithLocking($callback, $pid) { + $args = array_slice(func_get_args(), 2); + $locked = FALSE; + + if (variable_get('islandora_use_object_semaphores', FALSE)) { + $lock_period = variable_get('islandora_semaphore_period', 600); + if (!lock_acquire($pid, $lock_period)) { + // Waiting forever. + while (!lock_wait($pid) && lock_acquire($pid, $lock_period)) { + break; + } + } + $locked = TRUE; + } + + if ($locked) { + try { + $to_return = call_user_func_array(array($this, "parent::$callback"), $args); + } + catch (Exception $e) { + // Release the lock in event of exception. + lock_release($pid); + throw $e; + } + lock_release($pid); + return $to_return; + } + else { + return call_user_func_array(array($this, "parent::$callback"), $args); + } + } } class IslandoraSimpleCache extends SimpleCache {} diff --git a/islandora.install b/islandora.install index 075e81a2..e1dc340d 100644 --- a/islandora.install +++ b/islandora.install @@ -55,6 +55,8 @@ function islandora_uninstall() { 'islandora_namespace_restriction_enforced', 'islandora_pids_allowed', 'islandora_risearch_use_itql_when_necessary', + 'islandora_use_object_semaphores', + 'islandora_semaphore_period', ); array_walk($variables, 'variable_del'); } From 6b229129f172564dd9e5cd7f2c4843fc4c2fd09d Mon Sep 17 00:00:00 2001 From: willtp87 Date: Thu, 3 Dec 2015 11:42:45 -0400 Subject: [PATCH 2/4] Better validation for semaphores. --- includes/admin.form.inc | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/includes/admin.form.inc b/includes/admin.form.inc index c233be2c..42b197ec 100644 --- a/includes/admin.form.inc +++ b/includes/admin.form.inc @@ -72,7 +72,6 @@ function islandora_repository_admin(array $form, array &$form_state) { '#title' => t('Time to Claim Objects for'), '#default_value' => variable_get('islandora_semaphore_period', 600), '#description' => t('Time in seconds to claim objects for modification.'), - '#required' => TRUE, '#states' => array( 'invisible' => array( ':input[name="islandora_use_object_semaphores"]' => array('checked' => FALSE), @@ -141,6 +140,21 @@ function islandora_repository_admin(array $form, array &$form_state) { return system_settings_form($form); } +/** + * Validate the admin form. + */ +function islandora_repository_admin_validate($form, &$form_state) { + // Only validate semaphore period if semaphores are enabled. + if ($form_state['values']['islandora_use_object_semaphores']) { + if ($form_state['values']['islandora_semaphore_period']) { + element_validate_integer_positive($form['islandora_tabs']['islandora_general']['islandora_semaphore_period'], $form_state); + } + else { + form_set_error('islandora_semaphore_period', t('Time to Claim Objects for must not be empty if Make Processes Claim Objects for Modification is checked.')); + } + } +} + /** * Gets a message which describes if the repository is accessible. * From 3c1dda02d58bd63578be588e9bb0b6f1cf498405 Mon Sep 17 00:00:00 2001 From: willtp87 Date: Fri, 4 Dec 2015 14:14:55 -0400 Subject: [PATCH 3/4] Better doc for claim time. --- includes/admin.form.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/admin.form.inc b/includes/admin.form.inc index 42b197ec..18b9bb4e 100644 --- a/includes/admin.form.inc +++ b/includes/admin.form.inc @@ -71,7 +71,7 @@ function islandora_repository_admin(array $form, array &$form_state) { '#type' => 'textfield', '#title' => t('Time to Claim Objects for'), '#default_value' => variable_get('islandora_semaphore_period', 600), - '#description' => t('Time in seconds to claim objects for modification.'), + '#description' => t('Maximum time in seconds to claim objects for modification.'), '#states' => array( 'invisible' => array( ':input[name="islandora_use_object_semaphores"]' => array('checked' => FALSE), From db8062fda9c4a58cb94176016e36bee8ca449f08 Mon Sep 17 00:00:00 2001 From: willtp87 Date: Mon, 7 Dec 2015 14:49:18 -0400 Subject: [PATCH 4/4] Better handling when we can not get a lock. --- includes/tuque_wrapper.inc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/includes/tuque_wrapper.inc b/includes/tuque_wrapper.inc index 5f09985b..f075c402 100644 --- a/includes/tuque_wrapper.inc +++ b/includes/tuque_wrapper.inc @@ -493,10 +493,9 @@ class IslandoraFedoraApiM extends FedoraApiM { if (variable_get('islandora_use_object_semaphores', FALSE)) { $lock_period = variable_get('islandora_semaphore_period', 600); - if (!lock_acquire($pid, $lock_period)) { - // Waiting forever. - while (!lock_wait($pid) && lock_acquire($pid, $lock_period)) { - break; + while (!lock_acquire($pid, $lock_period)) { + // Wait for the lock to be free. In the worst case forever. + while (lock_wait($pid)) { } } $locked = TRUE;