Merge pull request #276 from acelaya/feature/locking

Feature/locking
This commit is contained in:
Alejandro Celaya 2018-11-17 14:33:58 +01:00 committed by GitHub
commit 67e465c479
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 162 additions and 54 deletions

View file

@ -25,6 +25,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
#### Fixed
* [#271](https://github.com/shlinkio/shlink/issues/271) Fixed memory leak produced when processing high amounts of visits at the same time.
* [#272](https://github.com/shlinkio/shlink/issues/272) Fixed errors produced when trying to process visits multiple times in parallel, by using a lock which ensures only one instance is run at a time.
## 1.14.0 - 2018-11-16

View file

@ -32,6 +32,7 @@
"roave/security-advisories": "dev-master",
"symfony/console": "^4.1",
"symfony/filesystem": "^4.1",
"symfony/lock": "^4.1",
"symfony/process": "^4.1",
"theorchard/monolog-cascade": "^0.4",
"zendframework/zend-config": "^3.0",

View file

@ -0,0 +1,25 @@
<?php
declare(strict_types=1);
use Symfony\Component\Lock;
use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory;
return [
'locks' => [
'locks_dir' => __DIR__ . '/../../data/locks',
],
'dependencies' => [
'factories' => [
Lock\Store\FlockStore::class => ConfigAbstractFactory::class,
Lock\Factory::class => ConfigAbstractFactory::class,
],
],
ConfigAbstractFactory::class => [
Lock\Store\FlockStore::class => ['config.locks.locks_dir'],
Lock\Factory::class => [Lock\Store\FlockStore::class],
],
];

2
data/locks/.gitignore vendored Normal file
View file

@ -0,0 +1,2 @@
*
!.gitignore

View file

@ -9,6 +9,7 @@ use Shlinkio\Shlink\Common\Service\PreviewGenerator;
use Shlinkio\Shlink\Core\Service;
use Shlinkio\Shlink\Rest\Service\ApiKeyService;
use Symfony\Component\Console\Application;
use Symfony\Component\Lock;
use Zend\I18n\Translator\Translator;
use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory;
@ -68,6 +69,7 @@ return [
Command\Visit\ProcessVisitsCommand::class => [
Service\VisitService::class,
IpLocationResolverInterface::class,
Lock\Factory::class,
'translator',
],
Command\Visit\UpdateDbCommand::class => [DbUpdater::class, 'translator'],

Binary file not shown.

View file

@ -1,8 +1,8 @@
msgid ""
msgstr ""
"Project-Id-Version: Shlink 1.0\n"
"POT-Creation-Date: 2018-11-12 21:01+0100\n"
"PO-Revision-Date: 2018-11-12 21:03+0100\n"
"POT-Creation-Date: 2018-11-17 14:29+0100\n"
"PO-Revision-Date: 2018-11-17 14:29+0100\n"
"Last-Translator: Alejandro Celaya <alejandro@alejandrocelaya.com>\n"
"Language-Team: \n"
"Language: es_ES\n"
@ -340,6 +340,17 @@ msgstr "Una etiqueta con nombre \"%s\" no ha sido encontrada"
msgid "Processes visits where location is not set yet"
msgstr "Procesa las visitas donde la localización no ha sido establecida aún"
#, php-format
msgid "There is already an instance of the \"%s\" command in execution"
msgstr "Ya existe una instancia del comando \"%s\" en ejecución"
#, php-format
msgid "Address located at \"%s\""
msgstr "Dirección localizada en \"%s\""
msgid "Finished processing all IPs"
msgstr "Finalizado el procesado de todas las IPs"
msgid "Ignored visit with no IP address"
msgstr "Ignorada visita sin dirección IP"
@ -349,16 +360,9 @@ msgstr "Procesando IP"
msgid "Ignored localhost address"
msgstr "Ignorada IP de localhost"
#, php-format
msgid "Address located at \"%s\""
msgstr "Dirección localizada en \"%s\""
msgid "An error occurred while locating IP. Skipped"
msgstr "Se produjo un error al localizar la IP. Ignorado"
msgid "Finished processing all IPs"
msgstr "Finalizado el procesado de todas las IPs"
msgid "Updates the GeoLite2 database file used to geolocate IP addresses"
msgstr ""
"Actualiza el fichero de base de datos de GeoLite2 usado para geolocalizar "

View file

@ -14,6 +14,7 @@ use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Style\SymfonyStyle;
use Symfony\Component\Lock\Factory as Locker;
use Zend\I18n\Translator\TranslatorInterface;
use function sprintf;
@ -33,69 +34,99 @@ class ProcessVisitsCommand extends Command
* @var TranslatorInterface
*/
private $translator;
/**
* @var Locker
*/
private $locker;
/**
* @var OutputInterface
*/
private $output;
public function __construct(
VisitServiceInterface $visitService,
IpLocationResolverInterface $ipLocationResolver,
Locker $locker,
TranslatorInterface $translator
) {
$this->visitService = $visitService;
$this->ipLocationResolver = $ipLocationResolver;
$this->translator = $translator;
$this->locker = $locker;
parent::__construct();
}
protected function configure(): void
{
$this->setName(self::NAME)
->setDescription(
$this->translator->translate('Processes visits where location is not set yet')
);
$this
->setName(self::NAME)
->setDescription($this->translator->translate('Processes visits where location is not set yet'));
}
protected function execute(InputInterface $input, OutputInterface $output): void
{
$this->visitService->locateVisits(function (Visit $visit) use ($output) {
if (! $visit->hasRemoteAddr()) {
$output->writeln(
sprintf('<comment>%s</comment>', $this->translator->translate('Ignored visit with no IP address')),
OutputInterface::VERBOSITY_VERBOSE
);
throw new IpCannotBeLocatedException('Ignored visit with no IP address');
}
$ipAddr = $visit->getRemoteAddr();
$output->write(sprintf('%s <fg=blue>%s</>', $this->translator->translate('Processing IP'), $ipAddr));
if ($ipAddr === IpAddress::LOCALHOST) {
$output->writeln(
sprintf(' [<comment>%s</comment>]', $this->translator->translate('Ignored localhost address'))
);
throw new IpCannotBeLocatedException('Ignored localhost address');
}
try {
return $this->ipLocationResolver->resolveIpLocation($ipAddr);
} catch (WrongIpException $e) {
$output->writeln(
sprintf(
' [<fg=red>%s</>]',
$this->translator->translate('An error occurred while locating IP. Skipped')
)
);
if ($output->isVerbose()) {
$this->getApplication()->renderException($e, $output);
}
throw new IpCannotBeLocatedException('An error occurred while locating IP', 0, $e);
}
}, function (VisitLocation $location) use ($output) {
$output->writeln(sprintf(
' [<info>' . $this->translator->translate('Address located at "%s"') . '</info>]',
$location->getCountryName()
));
});
$this->output = $output;
$io = new SymfonyStyle($input, $output);
$io->success($this->translator->translate('Finished processing all IPs'));
$lock = $this->locker->createLock(self::NAME);
if (! $lock->acquire()) {
$io->warning(sprintf(
$this->translator->translate('There is already an instance of the "%s" command in execution'),
self::NAME
));
return;
}
try {
$this->visitService->locateVisits(
[$this, 'getGeolocationDataForVisit'],
function (VisitLocation $location) use ($output) {
$output->writeln(sprintf(
' [<info>' . $this->translator->translate('Address located at "%s"') . '</info>]',
$location->getCountryName()
));
}
);
$io->success($this->translator->translate('Finished processing all IPs'));
} finally {
$lock->release();
}
}
public function getGeolocationDataForVisit(Visit $visit): array
{
if (! $visit->hasRemoteAddr()) {
$this->output->writeln(sprintf(
'<comment>%s</comment>',
$this->translator->translate('Ignored visit with no IP address')
), OutputInterface::VERBOSITY_VERBOSE);
throw new IpCannotBeLocatedException('Ignored visit with no IP address');
}
$ipAddr = $visit->getRemoteAddr();
$this->output->write(sprintf('%s <fg=blue>%s</>', $this->translator->translate('Processing IP'), $ipAddr));
if ($ipAddr === IpAddress::LOCALHOST) {
$this->output->writeln(
sprintf(' [<comment>%s</comment>]', $this->translator->translate('Ignored localhost address'))
);
throw new IpCannotBeLocatedException('Ignored localhost address');
}
try {
return $this->ipLocationResolver->resolveIpLocation($ipAddr);
} catch (WrongIpException $e) {
$this->output->writeln(
sprintf(
' [<fg=red>%s</>]',
$this->translator->translate('An error occurred while locating IP. Skipped')
)
);
if ($this->output->isVerbose()) {
$this->getApplication()->renderException($e, $this->output);
}
throw new IpCannotBeLocatedException('An error occurred while locating IP', 0, $e);
}
}
}

View file

@ -19,9 +19,11 @@ use Shlinkio\Shlink\Core\Service\VisitService;
use Symfony\Component\Console\Application;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Tester\CommandTester;
use Symfony\Component\Lock;
use Throwable;
use Zend\I18n\Translator\Translator;
use function array_shift;
use function sprintf;
class ProcessVisitsCommandTest extends TestCase
{
@ -37,15 +39,31 @@ class ProcessVisitsCommandTest extends TestCase
* @var ObjectProphecy
*/
private $ipResolver;
/**
* @var ObjectProphecy
*/
private $locker;
/**
* @var ObjectProphecy
*/
private $lock;
public function setUp()
{
$this->visitService = $this->prophesize(VisitService::class);
$this->ipResolver = $this->prophesize(IpApiLocationResolver::class);
$this->locker = $this->prophesize(Lock\Factory::class);
$this->lock = $this->prophesize(Lock\LockInterface::class);
$this->lock->acquire()->willReturn(true);
$this->lock->release()->will(function () {
});
$this->locker->createLock(Argument::type('string'))->willReturn($this->lock->reveal());
$command = new ProcessVisitsCommand(
$this->visitService->reveal(),
$this->ipResolver->reveal(),
$this->locker->reveal(),
Translator::factory([])
);
$app = new Application();
@ -160,4 +178,28 @@ class ProcessVisitsCommandTest extends TestCase
$resolveIpLocation->shouldHaveBeenCalledOnce();
}
}
/**
* @test
*/
public function noActionIsPerformedIfLockIsAcquired()
{
$this->lock->acquire()->willReturn(false);
$locateVisits = $this->visitService->locateVisits(Argument::cetera())->will(function () {
});
$resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([]);
$this->commandTester->execute([
'command' => 'visit:process',
], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]);
$output = $this->commandTester->getDisplay();
$this->assertContains(
sprintf('There is already an instance of the "%s" command', ProcessVisitsCommand::NAME),
$output
);
$locateVisits->shouldNotHaveBeenCalled();
$resolveIpLocation->shouldNotHaveBeenCalled();
}
}