Skip to content

Commit

Permalink
fix(federation): Fix the handling of retrying OCM notifications when …
Browse files Browse the repository at this point in the history
…the remote was not available

Signed-off-by: Joas Schilling <coding@schilljs.com>
  • Loading branch information
nickvergessen committed Mar 12, 2024
1 parent 1ebf65d commit 14d1eeb
Show file tree
Hide file tree
Showing 7 changed files with 367 additions and 128 deletions.
3 changes: 2 additions & 1 deletion appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ And in the works for the [coming versions](https://github.com/nextcloud/spreed/m
]]></description>

<version>19.0.0-beta.1</version>
<version>19.0.0-beta.1.1</version>
<licence>agpl</licence>

<author>Daniel Calviño Sánchez</author>
Expand Down Expand Up @@ -65,6 +65,7 @@ And in the works for the [coming versions](https://github.com/nextcloud/spreed/m
<job>OCA\Talk\BackgroundJob\Reminder</job>
<job>OCA\Talk\BackgroundJob\RemoveEmptyRooms</job>
<job>OCA\Talk\BackgroundJob\ResetAssignedSignalingServer</job>
<job>OCA\Talk\BackgroundJob\RetryNotificationsJob</job>
</background-jobs>

<repair-steps>
Expand Down
100 changes: 0 additions & 100 deletions lib/BackgroundJob/RetryJob.php

This file was deleted.

49 changes: 49 additions & 0 deletions lib/BackgroundJob/RetryNotificationsJob.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2024 Joas Schilling <coding@schilljs.com>
*
* @author Joas Schilling <coding@schilljs.com>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace OCA\Talk\BackgroundJob;

use OCA\Talk\Federation\BackendNotifier;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\TimedJob;

/**
* Retry to send OCM notifications
*/
class RetryNotificationsJob extends TimedJob {
public function __construct(
private BackendNotifier $backendNotifier,
ITimeFactory $timeFactory,
) {
parent::__construct($timeFactory);

// Every time the jobs run
$this->setInterval(1);
}

protected function run($argument): void {
$this->backendNotifier->retrySendingFailedNotifications($this->time->getDateTime());
}
}
109 changes: 82 additions & 27 deletions lib/Federation/BackendNotifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,13 @@
use OCA\Talk\Config;
use OCA\Talk\Exceptions\RoomHasNoModeratorException;
use OCA\Talk\Model\Attendee;
use OCA\Talk\Model\RetryNotification;
use OCA\Talk\Model\RetryNotificationMapper;
use OCA\Talk\Room;
use OCP\App\IAppManager;
use OCP\AppFramework\Http;
use OCP\AppFramework\Services\IAppConfig;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\IJobList;
use OCP\DB\Exception;
use OCP\Federation\ICloudFederationFactory;
Expand Down Expand Up @@ -62,6 +65,8 @@ public function __construct(
private IAppManager $appManager,
private Config $talkConfig,
private IAppConfig $appConfig,
private RetryNotificationMapper $retryNotificationMapper,
private ITimeFactory $timeFactory,
) {
}

Expand Down Expand Up @@ -344,29 +349,11 @@ public function sendMessageUpdate(
$this->sendUpdateToRemote($remote, $notification);
}

/**
* @param string $remote
* @param array{notificationType: string, resourceType: string, providerId: string, notification: array} $data
* @param int $try
* @return void
* @internal Used to send retries in background jobs
*/
public function sendUpdateDataToRemote(string $remote, array $data, int $try): void {
$notification = $this->cloudFederationFactory->getCloudFederationNotification();
$notification->setMessage(
$data['notificationType'],
$data['resourceType'],
$data['providerId'],
$data['notification']
);
$this->sendUpdateToRemote($remote, $notification, $try);
}

protected function sendUpdateToRemote(string $remote, ICloudFederationNotification $notification, int $try = 0): void {
protected function sendUpdateToRemote(string $remote, ICloudFederationNotification $notification, int $try = 0): bool {
try {
$response = $this->federationProviderManager->sendCloudNotification($remote, $notification);
if ($response->getStatusCode() === Http::STATUS_CREATED) {
return;
return true;
}

$this->logger->warning("Failed to send notification for share from $remote, received status code {code}\n{body}", [
Expand All @@ -377,14 +364,82 @@ protected function sendUpdateToRemote(string $remote, ICloudFederationNotificati
$this->logger->error("Failed to send notification for share from $remote, received OCMProviderException", ['exception' => $e]);
}

$this->jobList->add(
RetryJob::class,
[
'remote' => $remote,
'data' => json_encode($notification->getMessage(), JSON_THROW_ON_ERROR),
'try' => $try,
]
if ($try === 0) {
$now = $this->timeFactory->getTime();
$now += $this->getRetryDelay(1);

$retryNotification = new RetryNotification();
$retryNotification->setRemoteServer($remote);
$retryNotification->setNumAttempts(1);
$retryNotification->setNextRetry($this->timeFactory->getDateTime('@' . $now));

$data = $notification->getMessage();
$retryNotification->setNotificationType($data['notificationType']);
$retryNotification->setResourceType($data['resourceType']);
$retryNotification->setProviderId($data['providerId']);
$retryNotification->setNotification($data['notification']);

$this->retryNotificationMapper->insert($retryNotification);
}

return false;
}

public function retrySendingFailedNotifications(\DateTimeInterface $dueDateTime): void {
$retryNotifications = $this->retryNotificationMapper->getAllDue($dueDateTime);

foreach ($retryNotifications as $retryNotification) {
$this->retrySendingFailedNotification($retryNotification);
}
}

protected function retrySendingFailedNotification(RetryNotification $retryNotification): void {
$notification = $this->cloudFederationFactory->getCloudFederationNotification();
$notification->setMessage(
$retryNotification->getNotificationType(),
$retryNotification->getResourceType(),
$retryNotification->getProviderId(),
json_decode($retryNotification->getNotification(), true, flags: JSON_THROW_ON_ERROR),
);

$success = $this->sendUpdateToRemote($retryNotification->getRemoteServer(), $notification, $retryNotification->getNumAttempts());

if ($success) {
$this->retryNotificationMapper->delete($retryNotification);
} elseif ($retryNotification->getNumAttempts() === RetryNotification::MAX_NUM_ATTEMPTS) {
$this->logger->error('Failed to send notification to ' . $retryNotification->getRemoteServer() . ' ' . RetryNotification::MAX_NUM_ATTEMPTS . ' times, giving up!');
$this->retryNotificationMapper->delete($retryNotification);
} else {
$retryNotification->setNumAttempts($retryNotification->getNumAttempts() + 1);

$now = $this->timeFactory->getTime();
$now += $this->getRetryDelay($retryNotification->getNumAttempts());

$retryNotification->setNextRetry($this->timeFactory->getDateTime('@' . $now));
$this->retryNotificationMapper->update($retryNotification);
}
}

/**
* First 5 attempts are retried on the next cron run.
* Attempts 6-10 we back off to cover slightly longer maintenance/downtimes (5 minutes * per attempt)
* And the last tries 11-20 are retried with ~6 hours delay
*
* This means the last retry is after ~84 hours so a downtime from Friday to Monday would be covered
*/
protected function getRetryDelay(int $attempt): int {
if ($attempt < 5) {
// Retry after "attempt" minutes
return 5 * 60;
}

if ($attempt > 10) {
// Retry after 8 hours
return 8 * 3600;
}

// Retry after "attempt" * 5 minutes
return $attempt * 5 * 60;
}

protected function prepareRemoteUrl(string $remote): string {
Expand Down
Loading

0 comments on commit 14d1eeb

Please sign in to comment.