Differential D20087 Diff 47996 src/infrastructure/util/PhabricatorMetronome.php

Changeset View

Standalone View

src/infrastructure/util/PhabricatorMetronome.php

This file was added.

				<?php

				/**
				* Tick at a given frequency with a specifiable offset.
				*
				* One use case for this is to flatten out load spikes caused by periodic
				* service calls. Give each host a metronome that ticks at the same frequency,
				* but with different offsets. Then, have hosts make service calls only after
				* their metronome ticks. This spreads service calls out evenly more quickly
				* and more predictably than adding random jitter.
				*/
				final class PhabricatorMetronome
				extends Phobject {

				private $offset = 0;
				private $frequency;

				public function setOffset($offset) {
				if (!is_int($offset)) {
				throw new Exception(pht('Metronome offset must be an integer.'));
				}

				if ($offset < 0) {
				throw new Exception(pht('Metronome offset must be 0 or more.'));
				}

				// We're not requiring that the offset be smaller than the frequency. If
				// the offset is larger, we'll just clamp it to the frequency before we
				// use it. This allows the offset to be configured before the frequency
				// is configured, which is useful for using a hostname as an offset seed.

				$this->offset = $offset;

				return $this;
				}

				public function setFrequency($frequency) {
				if (!is_int($frequency)) {
				throw new Exception(pht('Metronome frequency must be an integer.'));
				}

				if ($frequency < 1) {
				throw new Exception(pht('Metronome frequency must be 1 or more.'));
				}

				$this->frequency = $frequency;

				return $this;
				}

				public function setOffsetFromSeed($seed) {
				$offset = PhabricatorHash::digestToRange($seed, 0, PHP_INT_MAX);
				return $this->setOffset($offset);
				amckinleyUnsubmitted Not Done Inline Actions Is this just to enforce setting a frequency before setting an offset? If so, why doesn't `setOffset()` have the same code? amckinley: Is this just to enforce setting a frequency before setting an offset? If so, why doesn't…
				epriestleyAuthorUnsubmitted Done Inline Actions Initially I was doing `$offset = $offset % $frequency` but that prevented setting the offset before the frequency, and it's useful to return a metronome with a pre-seeded offset so I got rid of that : `function newMetronomeForThisServer() { return metronome->setOffsetFromSeed(php_uname('n')); }`. This code now has no effect except preventing the use case I actually want (setting offset before frequency). I removed it locally but neglected to update the change. epriestley: Initially I was doing `$offset = $offset % $frequency` but that prevented setting the offset…
				}
				amckinleyUnsubmitted Not Done Inline Actions Wow this is a cool function! amckinley: Wow this is a cool function! howneatisthat
				epriestleyAuthorUnsubmitted Done Inline Actions (It's slightly biased, but okay for the current use cases of "profile pictures" and "make servers kind of not call services all at the same time".) epriestley: (It's slightly biased, but okay for the current use cases of "profile pictures" and "make…

				public function getFrequency() {
				if ($this->frequency === null) {
				throw new PhutilInvalidStateException('setFrequency');
				}
				return $this->frequency;
				}

				public function getOffset() {
				$frequency = $this->getFrequency();
				return ($this->offset % $frequency);
				}

				amckinleyUnsubmitted Not Done Inline Actions I agree that this implementation makes the most sense, but because this code is intended to produce a uniform distribution and because I spent a lot of money to get a CS degree, I feel obligated to point out that certain values for frequency (in particular, powers of 2) can end up generating a very non-uniform distribution of offsets that is significantly worse than the birthday paradox would imply. There's also nothing we can really do in the general sense to fix this other than something like rounding `$frequency` up to the closest prime number or something equally silly. If we wanted to totally change the interface for this, we could require that you initialize a metronome with an array of all the objects that will be using it, and assign offsets internally to enforce the most uniform-possible distribution, but that would make this way more unwieldy. amckinley: I agree that this implementation makes the most sense, but because this code is intended to…
				epriestleyAuthorUnsubmitted Done Inline Actions Yeah -- `digestToRange()` also uses a simple `%` and is biased (although I think the low-order bits of `sha1` are not particularly weak?) and on top of everything else this code also doesn't do the right thing with leap seconds. This is definitely aimed at being "better than random jitter", not "ready to launch into orbit". epriestley: Yeah -- `digestToRange()` also uses a simple `%` and is biased (although I //think// the low…
				public function getNextTickAfter($epoch) {
				$frequency = $this->getFrequency();
				$offset = $this->getOffset();

				$remainder = ($epoch % $frequency);

				if ($remainder < $offset) {
				return ($epoch - $remainder) + $offset;
				} else {
				return ($epoch - $remainder) + $frequency + $offset;
				}
				}

				public function didTickBetween($min, $max) {
				if ($max < $min) {
				throw new Exception(
				pht(
				'Maximum tick window must not be smaller than minimum tick window.'));
				}

				$next = $this->getNextTickAfter($min);
				return ($next <= $max);
				}

				}