From 32f3de92a74af9578408793aeb312ecc35f79fb7 Mon Sep 17 00:00:00 2001 From: "Matthew J. Mucklo" Date: Fri, 12 Jul 2019 00:27:13 -0700 Subject: [PATCH] fixes for issue #98 - make microtime usage super consistent --- CHANGELOG.md | 2 + .../Compiler/WorkerCompilerPass.php | 2 +- DependencyInjection/Configuration.php | 2 +- Doctrine/DoctrineRunManager.php | 2 +- Manager/RunManager.php | 15 +++----- Manager/WorkerManager.php | 4 +- Model/MicrotimeTrait.php | 2 +- Model/Worker.php | 8 ++-- ORM/JobManager.php | 10 ++--- Redis/JobManager.php | 10 ++--- Resources/docker/php/ubuntu/Dockerfile | 21 ++++++++++ Run/Loop.php | 10 ++--- Tests/Doctrine/DoctrineJobManagerTest.php | 6 +-- Tests/Manager/WorkerManagerTest.php | 8 ++-- Tests/Model/WorkerTest.php | 8 ++-- Tests/Redis/JobManagerTest.php | 6 +-- Tests/Run/LoopTest.php | 1 - Util/Util.php | 38 +++++++++++++++++-- 18 files changed, 102 insertions(+), 53 deletions(-) create mode 100644 Resources/docker/php/ubuntu/Dockerfile diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b56c1e..0418daa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,5 @@ +### 5.1.0 + * Issue #98 fix microtime issues in locales that do not use '.' for the decimal point. ### 5.0.1 * Pulling in a Query Builder fix from a fork https://github.com/quitoque/QueueBundle/commit/00f788e84765df2dcb81a19907463dd8eace566d ### 5.0.0 diff --git a/DependencyInjection/Compiler/WorkerCompilerPass.php b/DependencyInjection/Compiler/WorkerCompilerPass.php index 8a1a556..1905355 100644 --- a/DependencyInjection/Compiler/WorkerCompilerPass.php +++ b/DependencyInjection/Compiler/WorkerCompilerPass.php @@ -168,7 +168,7 @@ protected function addLiveJobs(ContainerBuilder $container) /** * @param $managerType * - * @return null|string + * @return string|null */ protected function getDirectory($managerType) { diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index e2d2db3..b8da38b 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -233,7 +233,7 @@ protected function addPriority() // BC layer for symfony/config 4.1 and older $rootNode = $treeBuilder->root('priority'); } - $rootNode + $rootNode ->addDefaultsIfNotSet() ->children() ->integerNode('max') diff --git a/Doctrine/DoctrineRunManager.php b/Doctrine/DoctrineRunManager.php index 1685b89..bd431af 100644 --- a/Doctrine/DoctrineRunManager.php +++ b/Doctrine/DoctrineRunManager.php @@ -39,7 +39,7 @@ public function getRepository() } /** - * @return null|string + * @return string|null */ public function getRunArchiveClass() { diff --git a/Manager/RunManager.php b/Manager/RunManager.php index 06e2066..dbf7a51 100644 --- a/Manager/RunManager.php +++ b/Manager/RunManager.php @@ -68,9 +68,10 @@ public function recordHeartbeat(Run $run, $start, Job $job = null) $jobId = $job->getId(); } - $run->setLastHeartbeatAt(Util::getMicrotimeDateTime()); + $heartbeat = microtime(true); + $run->setLastHeartbeatAt(Util::getMicrotimeFloatDateTime($heartbeat)); $run->setCurrentJobId($jobId); - $run->setElapsed(microtime(true) - $start); + $run->setElapsed($heartbeat - $start); $this->persistRun($run); } @@ -107,10 +108,7 @@ public function runStart($start, $maxCount = null, $duration = null, $processTim $runClass = $this->getRunClass(); /** @var Run $run */ $run = new $runClass(); - $startDate = \DateTime::createFromFormat('U.u', $formattedStart = number_format($start, 6, '.', ''), new \DateTimeZone(date_default_timezone_get())); - if (false === $startDate) { - throw new \RuntimeException("Could not create date from $start formatted as $formattedStart"); - } + $startDate = Util::getMicrotimeFloatDateTime($start); $run->setLastHeartbeatAt($startDate); $run->setStartedAt($startDate); if (null !== $maxCount) { @@ -135,10 +133,7 @@ public function runStart($start, $maxCount = null, $duration = null, $processTim public function runStop(Run $run, $start) { $end = microtime(true); - $endedTime = \DateTime::createFromFormat('U.u', $end, new \DateTimeZone(date_default_timezone_get())); - if ($endedTime) { - $run->setEndedAt($endedTime); - } + $run->setEndedAt(Util::getMicrotimeFloatDateTime($end)); $run->setElapsed($end - $start); $this->persistRun($run, 'remove'); } diff --git a/Manager/WorkerManager.php b/Manager/WorkerManager.php index 27e1a5a..ca19716 100644 --- a/Manager/WorkerManager.php +++ b/Manager/WorkerManager.php @@ -91,7 +91,7 @@ public function log($level, $msg, array $context = []) * @param null $methodName * @param bool $prioritize * - * @return null|Job + * @return Job|null */ public function run($workerName = null, $methodName = null, $prioritize = true, $runId = null) { @@ -146,7 +146,7 @@ public function runJob(Job $job) /** @var Worker $worker */ $worker = $this->getWorker($job->getWorkerName()); $this->log('debug', "Start: {$job->getClassName()}->{$job->getMethod()}", $job->getArgs()); - $job->setStartedAt(Util::getMicrotimeDateTime()); + $job->setStartedAt(Util::getMicrotimeFloatDateTime($start)); $job->setMessage(null); $worker->setCurrentJob($job); $result = call_user_func_array(array($worker, $job->getMethod()), $job->getArgs()); diff --git a/Model/MicrotimeTrait.php b/Model/MicrotimeTrait.php index 0992d93..cf16d75 100644 --- a/Model/MicrotimeTrait.php +++ b/Model/MicrotimeTrait.php @@ -10,7 +10,7 @@ public function setWhenAt(\DateTime $whenAt) { parent::setWhenAt($whenAt); - return $this->setWhenUs(Util::getMicrotimeDecimalFormat($whenAt)); + return $this->setWhenUs(Util::getMicrotimeIntegerFormat($whenAt)); } /** diff --git a/Model/Worker.php b/Model/Worker.php index 9362542..1d7e0d3 100644 --- a/Model/Worker.php +++ b/Model/Worker.php @@ -48,13 +48,15 @@ public function getJobManager() public function at($time = null, $batch = false, $priority = null) { $timeU = $time; + $localeInfo = localeconv(); + $decimalPoint = isset($localeInfo['decimal_point']) ? $localeInfo['decimal_point'] : '.'; if (null === $time) { $timeU = Util::getMicrotimeStr(); - } elseif (false === strpos(strval($time), '.')) { - $timeU = strval($time).'.000000'; + } elseif (false === strpos(strval($time), $decimalPoint)) { + $timeU = strval($time).$decimalPoint.'000000'; } - $dateTime = \DateTime::createFromFormat('U.u', (string) $timeU, new \DateTimeZone(date_default_timezone_get())); + $dateTime = \DateTime::createFromFormat('U'.$decimalPoint.'u', (string) $timeU, new \DateTimeZone(date_default_timezone_get())); if (!$dateTime) { throw new \InvalidArgumentException("Invalid time: $time".($timeU != $time ? " - (micro: $timeU)" : '')); } diff --git a/ORM/JobManager.php b/ORM/JobManager.php index 0984048..8eb5aca 100644 --- a/ORM/JobManager.php +++ b/ORM/JobManager.php @@ -115,8 +115,8 @@ public function pruneArchivedJobs(\DateTime $olderThan) { return $this->removeOlderThan( $this->getJobArchiveClass(), - 'updatedAt', - $olderThan + 'updatedAt', + $olderThan ); } @@ -255,7 +255,7 @@ public function getJobQueryBuilder($workerName = null, $methodName = null, $prio protected function addStandardPredicates(QueryBuilder $queryBuilder, $status = BaseJob::STATUS_NEW) { $dateTime = Util::getMicrotimeDateTime(); - $decimal = Util::getMicrotimeDecimalFormat($dateTime); + $microtimeInteger = Util::getMicrotimeIntegerFormat($dateTime); $queryBuilder ->andWhere('j.status = :status')->setParameter('status', $status) @@ -267,7 +267,7 @@ protected function addStandardPredicates(QueryBuilder $queryBuilder, $status = B $queryBuilder->expr()->isNull('j.expiresAt'), $queryBuilder->expr()->gt('j.expiresAt', ':expiresAt') )) - ->setParameter('whenUs', $decimal) + ->setParameter('whenUs', $microtimeInteger) ->setParameter('expiresAt', $dateTime); } @@ -321,7 +321,7 @@ protected function findRefresh($id) * * @param \Dtc\QueueBundle\Model\Job $job * - * @return null|Job + * @return Job|null */ public function updateNearestBatch(\Dtc\QueueBundle\Model\Job $job) { diff --git a/Redis/JobManager.php b/Redis/JobManager.php index 6a635a4..4fac08a 100644 --- a/Redis/JobManager.php +++ b/Redis/JobManager.php @@ -26,7 +26,7 @@ protected function transferQueues() // Drains from WhenAt queue into Prioirty Queue $whenQueue = $this->getWhenQueueCacheKey(); $priorityQueue = $this->getPriorityQueueCacheKey(); - $microtime = Util::getMicrotimeDecimal(); + $microtime = Util::getMicrotimeInteger(); while ($jobId = $this->redis->zPopByMaxScore($whenQueue, $microtime)) { $jobMessage = $this->redis->get($this->getJobCacheKey($jobId)); if (is_string($jobMessage)) { @@ -85,7 +85,7 @@ protected function batchFoundJob(Job $job, $foundJobCacheKey, $foundJobMessage) $foundWhen = $foundJob->getWhenUs(); // Fix this using bcmath - $curtimeU = Util::getMicrotimeDecimal(); + $curtimeU = Util::getMicrotimeInteger(); $newFoundWhen = null; if (bccomp($foundWhen, $curtimeU) > 0 && bccomp($foundWhen, $when) >= 1) { $newFoundWhen = $when; @@ -183,7 +183,7 @@ public function prioritySave(\Dtc\QueueBundle\Model\Job $job) // Add to whenAt or priority queue? /// optimizaiton... $whenUs = $job->getWhenUs(); if (!$whenUs) { - $whenUs = Util::getMicrotimeDecimal(); + $whenUs = Util::getMicrotimeInteger(); $job->setWhenUs($whenUs); } @@ -287,7 +287,7 @@ public function getJob($workerName = null, $methodName = null, $prioritize = tru $jobId = $this->redis->zPop($queue); } else { $queue = $this->getWhenQueueCacheKey(); - $microtime = Util::getMicrotimeDecimal(); + $microtime = Util::getMicrotimeInteger(); $jobId = $this->redis->zPopByMaxScore($queue, $microtime); } @@ -315,7 +315,7 @@ protected function retrieveJob($jobId) public function getWaitingJobCount($workerName = null, $methodName = null) { - $microtime = Util::getMicrotimeDecimal(); + $microtime = Util::getMicrotimeInteger(); $count = $this->redis->zCount($this->getWhenQueueCacheKey(), 0, $microtime); if (null !== $this->maxPriority) { diff --git a/Resources/docker/php/ubuntu/Dockerfile b/Resources/docker/php/ubuntu/Dockerfile new file mode 100644 index 0000000..0924ab9 --- /dev/null +++ b/Resources/docker/php/ubuntu/Dockerfile @@ -0,0 +1,21 @@ +# For building PHP from source for debugging purposes (specifically used in troubleshooting Issue #98) +FROM ubuntu:19.10 +ENV DEBIAN_FRONTEND=noninteractive +RUN apt update +RUN apt install -y apt-utils +RUN apt install -y perl-modules +RUN apt upgrade -y +RUN apt install -y libfreetype6-dev +RUN apt install -y git +RUN apt install -y vim +RUN apt install -y libzip-dev libldap2-dev libxml2-dev libpng-dev libicu-dev libbz2-dev libtidy-dev +RUN apt install -y libmemcached-dev +RUN apt install -y libssl-dev +RUN apt install -y build-essential +RUN apt install -y autoconf automake re2c libtool bison +RUN apt install -y curl wget +RUN apt install -y netcat +RUN git clone https://git.php.net/repository/php-src.git +RUN apt install -y sendmail gawk +RUN apt install -y libcurl4-openssl-dev zlibc libgd-dev libfreetype6 libjpeg9 libgdbm-dev libsodium-dev mysql-common mysql-client postgresql-11 libreadline5 libreadline-dev +RUN apt install -y libsqlite3-dev diff --git a/Run/Loop.php b/Run/Loop.php index 83838be..86bfd4c 100644 --- a/Run/Loop.php +++ b/Run/Loop.php @@ -114,8 +114,8 @@ public function runJobById($start, $jobId) /** * @param float $start * @param int $nanoSleep - * @param null|int $maxCount - * @param null|int $duration + * @param int|null $maxCount + * @param int|null $duration */ public function runLoop($start, $workerName, $methodName, $maxCount, $duration = null, $nanoSleep = 500000000) { @@ -146,8 +146,8 @@ public function runLoop($start, $workerName, $methodName, $maxCount, $duration = /** * @param int $nanoSleep - * @param null|int $maxCount - * @param null|int $duration + * @param int|null $maxCount + * @param int|null $duration * * @throws \InvalidArgumentException */ @@ -192,7 +192,7 @@ protected function validateNanoSleep($nanoSleep) /** * @param int|null $duration * - * @return null|\DateTime + * @return \DateTime|null */ protected function getEndTime(Run $run, $duration) { diff --git a/Tests/Doctrine/DoctrineJobManagerTest.php b/Tests/Doctrine/DoctrineJobManagerTest.php index f1a63e7..5d7f014 100644 --- a/Tests/Doctrine/DoctrineJobManagerTest.php +++ b/Tests/Doctrine/DoctrineJobManagerTest.php @@ -873,7 +873,7 @@ public function testBatchJobs() $time1 = new \DateTime('@'.time()); $job2 = $worker->batchLater(0)->fibonacci(1); - $time2 = Util::getDateTimeFromDecimalFormat(Util::getMicrotimeDecimal()); + $time2 = Util::getDateTimeFromDecimalFormat(Util::getMicrotimeInteger()); self::assertEquals($job1, $job2); self::assertGreaterThanOrEqual($time1, $job2->getWhenAt()); @@ -890,9 +890,9 @@ public function testBatchJobs() $job1 = $worker->later(100)->setPriority(3)->fibonacci(1); $priority1 = $job1->getPriority(); - $time1 = Util::getDateTimeFromDecimalFormat(Util::getMicrotimeDecimal()); + $time1 = Util::getDateTimeFromDecimalFormat(Util::getMicrotimeInteger()); $job2 = $worker->batchLater(0)->setPriority(1)->fibonacci(1); - $time2 = Util::getDateTimeFromDecimalFormat(Util::getMicrotimeDecimal()); + $time2 = Util::getDateTimeFromDecimalFormat(Util::getMicrotimeInteger()); self::assertEquals($job1, $job2); self::assertNotEquals($priority1, $job2->getPriority()); diff --git a/Tests/Manager/WorkerManagerTest.php b/Tests/Manager/WorkerManagerTest.php index 800806b..b02f88e 100755 --- a/Tests/Manager/WorkerManagerTest.php +++ b/Tests/Manager/WorkerManagerTest.php @@ -63,7 +63,7 @@ public function testRun() self::assertEquals( BaseJob::STATUS_SUCCESS, $job->getStatus(), - 'Worker run should be successful' + 'Worker run should be successful' ); } @@ -80,7 +80,7 @@ public function testErrorRun() self::assertEquals( BaseJob::STATUS_EXCEPTION, $job->getStatus(), - 'Worker run should be not successful' + 'Worker run should be not successful' ); self::assertNotEmpty($job->getMessage(), 'Error message should not be empty'); } @@ -96,13 +96,13 @@ public function testRunJob() self::assertEquals( BaseJob::STATUS_SUCCESS, $job->getStatus(), - 'Worker run should be successful' + 'Worker run should be successful' ); self::assertEquals( '20: 6765', file_get_contents($this->worker->getFilename()), - 'Result of fibonacciFile() must match' + 'Result of fibonacciFile() must match' ); } } diff --git a/Tests/Model/WorkerTest.php b/Tests/Model/WorkerTest.php index 4b6c9e0..d57c141 100755 --- a/Tests/Model/WorkerTest.php +++ b/Tests/Model/WorkerTest.php @@ -140,7 +140,7 @@ protected function assertJob(Job $job, $time, $method, $priority = null) self::assertEquals( $time, $job->getWhenAt()->getTimestamp(), - 'Job start time should equals' + 'Job start time should equals' ); } @@ -148,7 +148,7 @@ protected function assertJob(Job $job, $time, $method, $priority = null) self::assertEquals( $priority, $job->getPriority(), - 'Priority should be the same.' + 'Priority should be the same.' ); } else { self::assertNull($job->getPriority(), 'Priority should be null'); @@ -157,12 +157,12 @@ protected function assertJob(Job $job, $time, $method, $priority = null) self::assertEquals( $this->worker->getName(), $job->getWorkerName(), - 'Worker should be the same' + 'Worker should be the same' ); self::assertEquals( $method, $job->getMethod(), - 'Worker method should be the same' + 'Worker method should be the same' ); // Make sure param gets saved diff --git a/Tests/Redis/JobManagerTest.php b/Tests/Redis/JobManagerTest.php index bf86074..3736b44 100644 --- a/Tests/Redis/JobManagerTest.php +++ b/Tests/Redis/JobManagerTest.php @@ -164,7 +164,7 @@ public function testBatchJobs() $job1 = $worker->later(100)->fibonacci(1); $time1 = new \DateTime('@'.time()); $job2 = $worker->batchLater(0)->fibonacci(1); - $time2 = Util::getDateTimeFromDecimalFormat(Util::getMicrotimeDecimal()); + $time2 = Util::getDateTimeFromDecimalFormat(Util::getMicrotimeInteger()); self::assertEquals($job1->getId(), $job2->getId()); self::assertGreaterThanOrEqual($time1, $job2->getWhenAt()); @@ -184,9 +184,9 @@ public function testBatchJobs() if (null !== self::$jobManager->getMaxPriority()) { $job1 = $worker->later(100)->setPriority(3)->fibonacci(1); $priority1 = $job1->getPriority(); - $time1 = Util::getDateTimeFromDecimalFormat(Util::getMicrotimeDecimal()); + $time1 = Util::getDateTimeFromDecimalFormat(Util::getMicrotimeInteger()); $job2 = $worker->batchLater(0)->setPriority(1)->fibonacci(1); - $time2 = Util::getDateTimeFromDecimalFormat(Util::getMicrotimeDecimal()); + $time2 = Util::getDateTimeFromDecimalFormat(Util::getMicrotimeInteger()); self::assertEquals($job1->getId(), $job2->getId()); self::assertNotEquals($priority1, $job2->getPriority()); diff --git a/Tests/Run/LoopTest.php b/Tests/Run/LoopTest.php index 0051cf3..3c9812a 100644 --- a/Tests/Run/LoopTest.php +++ b/Tests/Run/LoopTest.php @@ -2,7 +2,6 @@ namespace Dtc\QueueBundle\Tests\Run; -use Dtc\QueueBundle\Beanstalkd\Job; use Dtc\QueueBundle\Manager\WorkerManager; use Dtc\QueueBundle\ODM\JobManager; use Dtc\QueueBundle\Run\Loop; diff --git a/Util/Util.php b/Util/Util.php index af23940..655eea9 100644 --- a/Util/Util.php +++ b/Util/Util.php @@ -4,6 +4,13 @@ class Util { + private static $localeinfo; + + public static function resetLocaleInfo() + { + self::$localeinfo = null; + } + /** * Copies the members of obj1 that have public getters to obj2 if there exists a public setter in obj2 of the same suffix, it will also copy public variables. * @@ -101,14 +108,37 @@ public static function validateIntNull($varName, $var, $pow) public static function getMicrotimeStr() { + // issue #98 - try to use the locale specific way to split things apart + if (null === self::$localeinfo) { + self::$localeinfo = localeconv(); + print_r(self::$localeinfo); + } + + $decimalPoint = isset(self::$localeinfo['decimal_point']) ? self::$localeinfo['decimal_point'] : '.'; + echo "\n\ndecimalpoint: $decimalPoint\n"; $parts = explode(' ', microtime()); - $pos1 = strpos($parts[0], '.'); + print_r($parts); + print_r(microtime(true)); + $pos1 = strpos($parts[0], $decimalPoint); $timeU = $parts[1].'.'.substr($parts[0], $pos1 + 1, 6); return $timeU; } + public static function getMicrotimeFloatDateTime($microtime) + { + if (!is_float($microtime)) { + throw new \RuntimeException("Could not create date time expected-float microtime: $microtime"); + } + $result = \DateTime::createFromFormat('U.u', number_format($microtime, 6, '.', ''), new \DateTimeZone(date_default_timezone_get())); + if (!$result) { + throw new \RuntimeException("Could not create date time from float microtime: $microtime"); + } + + return $result; + } + /** * @throws \RuntimeException * @@ -124,12 +154,12 @@ public static function getMicrotimeDateTime() return $result; } - public static function getMicrotimeDecimal() + public static function getMicrotimeInteger() { - return self::getMicrotimeDecimalFormat(self::getMicrotimeDateTime()); + return self::getMicrotimeIntegerFormat(self::getMicrotimeDateTime()); } - public static function getMicrotimeDecimalFormat(\DateTime $dateTime) + public static function getMicrotimeIntegerFormat(\DateTime $dateTime) { $dateTimeUs = $dateTime->format('Uu'); $dateTimeUs = str_pad($dateTimeUs, 18, '0', STR_PAD_RIGHT);