Problem/Motivation

In Drupal\purge\Plugin\Purge\Purger\PurgerBase::getTimeHint() has the following condition:

if (!$this->hasRuntimeMeasurement()) {
   throw new \LogicException('Since ::hasRuntimeMeasurement() returns TRUE, ::getTimeHint() needs to be implemented!');
}

because of this exception, Purger plugins must implement ::hasRuntimeMeasurement() and set it to return TRUE even if they do not have a Runtime Measurement. Alternatively, they could override this method to bypass the exception.

Proposed resolution

The condition should be reversed so it will look like this:

if ($this->hasRuntimeMeasurement()) {
   throw new \LogicException('Since ::hasRuntimeMeasurement() returns TRUE, ::getTimeHint() needs to be implemented!');
}

Remaining tasks

  1. Write Patch

User interface changes

None.

API changes

This may cause some Purger plugins to throw an exception if they are doing something to bypass this logic.

Data model changes

None.

CommentFileSizeAuthor
#2 2744135-2.patch587 bytesjoshi.rohit100

Comments

davidwbarratt created an issue. See original summary.

joshi.rohit100’s picture

Status: Active » Needs review
StatusFileSize
new587 bytes

I faced the same problem while working on #2744413: KeyCDN purger

nielsvm’s picture

Status: Needs review » Fixed

Please, firstly, allow me to make you aware of the following commit that ends up in the next beta 8.x-3.0-beta5:

@@ -126,7 +126,7 @@ abstract class PurgerBase extends PluginBase implements PurgerInterface {
   */
   public function getTimeHint() {
     if (!$this->hasRuntimeMeasurement()) {
-      throw new \LogicException('Since ::hasRuntimeMeasurement() returns TRUE, ::getTimeHint() needs to be implemented!');
+      throw new \LogicException('Since ::hasRuntimeMeasurement() returns FALSE, ::getTimeHint() needs to be implemented!');
     }
 
     // Return the measured number of seconds, if stored of course.
because of this exception, Purger plugins must implement ::hasRuntimeMeasurement() and set it to return TRUE even if they do not have a Runtime Measurement. Alternatively, they could override this method to bypass the exception.

I think there's a misunderstanding hiding in your words: "do not have a Runtime Measurement". This seems to imply that purgers need to do anything for runtime measurement when returning TRUE. They do not, in fact, just setting it to TRUE will turn on all the magic sauce entirely.

No, what we in fact ask, is that the developer writing the purger... actively understands and thinks about the problem of runtime performance. This is also why there's no ::hasRuntimeMeasurement() {return TRUE;} in PurgerBase by default.

Let's have a look at its (just updated) definition:

  /**
   * Indicates whether your purger utilizes dynamic runtime measurement.
   *
   * Implementations of this method should simply return TRUE or FALSE but the
   * consequences of it have to be thouroughly understood. Either scenarios and
   * the resulting behavior explained:
   *
   * Dynamic runtime measurement enabled (TRUE):
   *  - A counter for your purger will be injected using ::setRuntimeMeasurement().
   *  - The injected counter will be returned by ::getRuntimeMeasurement().
   *  - RuntimeMeasurementInterface::start() is called before ::invalidate().
   *  - RuntimeMeasurementInterface::stopped() is called after ::invalidate().
   *  - Your ::getTimeHint() implementation is assumed to utilize the latest
   *    runtime measurement by calling RuntimeMeasurementInterface::get().
   *
   * In other words, returning TRUE will give you automatic adaptive performance
   * throthling based on data, gathered by automatically set performance
   * counters. When your purgers execute slow, less will be fed in the next
   * call to ::invalidate(). When it performs much better, it will incrementally
   * increase the amount it feeds, over the several next calls to ::invalidate.
   *
   * Dynamic runtime measurement disabled (FALSE):
   *  - No calls will be made to ::setRuntimeMeasurement().
   *  - No calls will be made to ::getRuntimeMeasurement().
   *  - Your ::getTimeHint() implementation will not use dynamic runtime
   *    tracking and carefully and responsibly define the right time hints.
   *
   * So returning FALSE, makes you responsible for the complex task of defining
   * a realistic time limit in which your code executes at all times. This is so
   * complex, because real-world circumstances (e.g. HTTP delays) can cause
   * sudden drops in productivity, which cannot come at the expense of Drupal's
   * overal stability. This is why Purge rather throttles its own work, than
   * letting things explode in front of the end-user. 
   *
   * @return bool
   *   Whether dynamic runtime measurement is used and should be injected.
   */
  public function hasRuntimeMeasurement();

So, you just need to return TRUE, but understand what's going on.

nielsvm’s picture

diff --git a/src/Plugin/Purge/Purger/PurgerBase.php b/src/Plugin/Purge/Purger/PurgerBase.php
index 76f5a6f..10f624c 100644
--- a/src/Plugin/Purge/Purger/PurgerBase.php
+++ b/src/Plugin/Purge/Purger/PurgerBase.php
@@ -121,7 +121,7 @@ abstract class PurgerBase extends PluginBase implements PurgerInterface {
   */
   public function getTimeHint() {
     if (!$this->hasRuntimeMeasurement()) {
-      throw new \LogicException('Since ::hasRuntimeMeasurement() returns FALSE, ::getTimeHint() needs to be implemented!');
+      throw new \LogicException('Since ::hasRuntimeMeasurement() returns FALSE, ::getTimeHint() needs to be implemented! Please read the PurgerCapacityDataInterface::hasRuntimeMeasurement() documentation.');
     }

Further clarifying and pointing developers in the right direction.

  • nielsvm committed 987b2b2 on 8.x-3.x
    Issue #2744135 by joshi.rohit100, davidwbarratt, nielsvm:...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.