Configure sample percentage to reduce server impact

Created on 12 February 2019, almost 6 years ago
Updated 10 August 2023, over 1 year ago

If we could configure a "sample_percentage" setting and the client side javascript would act on that percentage of views using Math.random, we could still determine content popularity on high traffic sites but significantly reduce backend impact.

Posting this early, I may have a stab at a patch later on.

Feature request
Status

Needs work

Version

4.0

Component

Code

Created by

🇳🇱Netherlands SqyD

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇳🇱Netherlands thomasdegraaff

    Patch attached.

    A 'Incident process ratio' setting has been added to the radioactivity field formatter. This setting is a number between 0 and 1 and defaults to 1. If it is set to 0.2 then one in the five incidents is posted to the backend. To accomplish this the incident process ratio is passed to the triggers.js where it is used to determine if a incident should be posted to the backend or not.

  • Status changed to Needs work almost 2 years ago
  • 🇳🇱Netherlands sutharsan

    The Math.random() function returns a floating-point, pseudo-random number in the range [ 0, 1) that is, from 0 (inclusive) up to but not including 1. So I believe the comparison Math.random() <= …is not correct.

    The readme document should be extended with a description of the new setting.

    What is the rationale of having the new setting per field instance? What is the advantage of having this per field instance? I see possible problems when for example comparing values.

  • 🇳🇱Netherlands thomasdegraaff

    Thanks for the feedback @Stuharsan. I'll fix the issues you mentioned.

    The setting should be site wide indeed. And the emitted value should be divided by the incident process ratio so the values submitted with different incident process ratios are comparable. I'll provide a patch later on.

  • 🇳🇱Netherlands thomasdegraaff

    Fixes broken link to admin settings page in previous patch, and removes debug console.log in javascript.

  • Status changed to Needs review over 1 year ago
  • The last submitted patch, 8: radioactivity-sample_percentage-3032540-8.patch, failed testing. View results
    - codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • The last submitted patch, 9: radioactivity-sample_percentage-3032540-9.patch, failed testing. View results
    - codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • Status changed to Needs work over 1 year ago
  • 🇳🇱Netherlands sutharsan

    Thanks for working on this issue!

    1. +++ b/README.md
      @@ -119,6 +119,14 @@ value is treated and how it behaves over time.
      +The emitted energy for a processed incident is divided by the incident process ratio to make the total emitted energy for a certain amount of incidents independent of the incident process ratio setting. This means the incident process ratio setting will not affect the average amount of energy a incident will emit.
      

      I don't understand this.

      1. Explain why lowering the value is good on high traffic sites. 2. Describe if it is ok to modify the value on an existing site.

      I think you mean multiply instead of divide (not implemented?). But personally I would not bother explaining it.

    2. +++ b/src/Form/SettingsForm.php
      @@ -0,0 +1,82 @@
      +      '#description' => $this->t('The ratio of incidents to process.') . t('Lower the value to reduce backend server load on high traffic sites.') . '<br/><br/>' . $this->requirementDescription() . '<br/>' . $this->t('For example: 1 or 0.5 or 0.005 etc.') . '<br/><br/>' . t('The emitted energy for a processed incident is divided by the incident process ratio to make the total emitted energy independent of the incident process ratio setting.'),
      +      '#default_value' => $this->config('radioactivity.settings')->get('incident_process_ratio') ?? 1,
      

      concatenating t function results will only work correct in left-to-right languages and not in right-to-left ones. Keep string on together. However
      separated strings may be concatenated as strings (= r-t-l safe)

      Only use $this->t() not t() in classes.

    3. +++ b/src/Form/SettingsForm.php
      @@ -0,0 +1,82 @@
      +    // Clear cache to make sure the javascript is not cached.
      

      Use short array syntax.

      The reference to javascript is confusing to me. The cache contains the field formatter (which in turn contains some JS).

    4. +++ b/src/Form/SettingsForm.php
      @@ -0,0 +1,82 @@
      +    Cache::invalidateTags(array('radioactivity_incident_process_ratio'));
      

      Use short array syntax!

  • 🇳🇱Netherlands thomasdegraaff

    @Sutharsan Thx for the feedback.

    To answer your questions:

    1. I'll use an example to explain it. Let's suppose an incident emits 10 energy. Let's suppose there have been 10 incidents. When the incident process ratio is 1, then these 10 incidents will emit 100 energy in total.

    Now the incident process ratio is set to 0.5.

    There have been 10 incidents again. Now statistically half of the 10 incidents will be processed, so that is 5 incidents. When each incident then emits 10 / 0.5 = 20 energy, then statistically the total energy emitted will be 5 x 20 = 100 energy in total.

    This way statistically 10 incidents will store the same amount of energy notwithstanding the incident process ratio that is set.

    The reason I explain this in the readme is to prevent users to change the emitted energy to do this correction themselves. Like as when someone sets the incident process ratio from 1 to 0.1 then this person might change the setting for emitted energy from 10 to 1 to not mess up the stats.

    2. Thanks for the explanation of the rationale of not concatenating translated strings. I'll fix these issues.

    3. I had an issue when testing the incident process ratio functionality that the actual process ratio would not change for a test page after changing the incident process ratio setting. Only after clearing the cache reloading the page would trigger incidents with the configured process ratio setting. The incident process ratio is passed to the javascript via drupalSettings and this seems to be cached so clearing the cache when the setting is changed seemed the right way to fix this.

    4. I'll fix this.

  • 🇳🇱Netherlands thomasdegraaff

    Fixes #13 issues

  • 🇳🇱Netherlands sutharsan
    1. +++ b/README.md
      @@ -119,6 +119,14 @@ value is treated and how it behaves over time.
      +The ratio of incidents to process. For example: 1 or 0.5 or 0.005 etc. Lower the value to reduce backend server load on high traffic sites.
      

      I would add an example: "For example with a process ration value of 0.2, every fifth page hit (on average) will trigger an incident. The emitted energy per incident is the configured value multiplied by 5."

    2. +++ b/src/Plugin/Field/FieldFormatter/RadioactivityReferenceEmitter.php
      @@ -125,6 +186,9 @@ class RadioactivityReferenceEmitter extends RadioactivityReferenceFormatterBase
      +    // Set custom cache tag to invalidate on incident process ratio config change.
      

      Comment is longer than 80 chars.

  • 🇮🇳India Akram Khan Cuttack, Odisha

    added updated patch and addressed #17

  • Status changed to Needs review over 1 year ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.0.5 + Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    73 pass, 1 fail
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.0.5 + Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    73 pass, 1 fail
  • 🇮🇳India Akram Khan Cuttack, Odisha
  • The last submitted patch, 16: radioactivity-sample_percentage-3032540-16.patch, failed testing. View results
    - codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • The last submitted patch, 18: 3032540-18.patch, failed testing. View results
    - codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • Status changed to Needs work over 1 year ago
  • 🇳🇱Netherlands thomasdegraaff

    Patch adds default incident process ratio setting of 1.

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.1 & MySQL 8
    last update over 1 year ago
    73 pass, 1 fail
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.0.5 + Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    73 pass, 1 fail
  • 🇳🇱Netherlands thomasdegraaff

    I can't get the test to pass, and fail to understand what goes wrong. It's quite hard to debug because there's no error messages showing what goes wrong. Maybe someone more experienced in writing functional tests can help?

Production build 0.71.5 2024