Schedule session index cleanup task only once. (#75661)

elastic · Sep 16, 2020 · 5f34d74 · 5f34d74
1 parent 0a58d51
commit 5f34d74
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 16 deletions.
diff --git a/x-pack/plugins/security/server/session_management/session_management_service.test.ts b/x-pack/plugins/security/server/session_management/session_management_service.test.ts
@@ -147,7 +147,9 @@ describe('SessionManagementService', () => {
       mockStatusSubject.next({ scheduleRetry: mockScheduleRetry });
       await nextTick();
       expect(mockSessionIndexInitialize).toHaveBeenCalledTimes(2);
-      expect(mockTaskManager.ensureScheduled).toHaveBeenCalledTimes(2);
+
+      // Session index task shouldn't be scheduled twice due to TM issue.
+      expect(mockTaskManager.ensureScheduled).toHaveBeenCalledTimes(1);
 
       expect(mockScheduleRetry).not.toHaveBeenCalled();
     });
@@ -199,16 +201,8 @@ describe('SessionManagementService', () => {
       expect(mockTaskManager.get).toHaveBeenCalledWith(SESSION_INDEX_CLEANUP_TASK_NAME);
 
       expect(mockTaskManager.remove).not.toHaveBeenCalled();
-
-      expect(mockTaskManager.ensureScheduled).toHaveBeenCalledTimes(1);
-      expect(mockTaskManager.ensureScheduled).toHaveBeenCalledWith({
-        id: SESSION_INDEX_CLEANUP_TASK_NAME,
-        taskType: SESSION_INDEX_CLEANUP_TASK_NAME,
-        scope: ['security'],
-        schedule: { interval: '3600s' },
-        params: {},
-        state: {},
-      });
+      // No need to schedule a task if Task Manager says it's already scheduled.
+      expect(mockTaskManager.ensureScheduled).not.toHaveBeenCalled();
     });
 
     it('schedules retry if index initialization fails', async () => {
@@ -224,11 +218,12 @@ describe('SessionManagementService', () => {
       expect(mockTaskManager.ensureScheduled).toHaveBeenCalledTimes(1);
       expect(mockScheduleRetry).toHaveBeenCalledTimes(1);
 
-      // Still fails.
+      // Still fails, but cleanup task is scheduled already
+      mockTaskManager.get.mockResolvedValue({ schedule: { interval: '3600s' } } as any);
       mockStatusSubject.next({ scheduleRetry: mockScheduleRetry });
       await nextTick();
       expect(mockSessionIndexInitialize).toHaveBeenCalledTimes(2);
-      expect(mockTaskManager.ensureScheduled).toHaveBeenCalledTimes(2);
+      expect(mockTaskManager.ensureScheduled).toHaveBeenCalledTimes(1);
       expect(mockScheduleRetry).toHaveBeenCalledTimes(2);
 
       // And finally succeeds, retry is not scheduled.
@@ -237,7 +232,7 @@ describe('SessionManagementService', () => {
       mockStatusSubject.next({ scheduleRetry: mockScheduleRetry });
       await nextTick();
       expect(mockSessionIndexInitialize).toHaveBeenCalledTimes(3);
-      expect(mockTaskManager.ensureScheduled).toHaveBeenCalledTimes(3);
+      expect(mockTaskManager.ensureScheduled).toHaveBeenCalledTimes(1);
       expect(mockScheduleRetry).toHaveBeenCalledTimes(2);
     });
 

diff --git a/x-pack/plugins/security/server/session_management/session_management_service.ts b/x-pack/plugins/security/server/session_management/session_management_service.ts
@@ -47,6 +47,7 @@ export class SessionManagementService {
   private statusSubscription?: Subscription;
   private sessionIndex!: SessionIndex;
   private config!: ConfigType;
+  private isCleanupTaskScheduled = false;
 
   constructor(private readonly logger: Logger) {}
 
@@ -124,7 +125,12 @@ export class SessionManagementService {
 
     // Check if currently scheduled task is scheduled with the correct interval.
     const cleanupInterval = `${this.config.session.cleanupInterval.asSeconds()}s`;
-    if (currentTask && currentTask.schedule?.interval !== cleanupInterval) {
+    if (currentTask) {
+      if (currentTask.schedule?.interval === cleanupInterval) {
+        this.logger.debug('Session index cleanup task is already scheduled.');
+        return;
+      }
+
       this.logger.debug(
         'Session index cleanup interval has changed, the cleanup task will be rescheduled.'
       );
@@ -139,6 +145,13 @@ export class SessionManagementService {
           throw err;
         }
       }
+    } else if (this.isCleanupTaskScheduled) {
+      // WORKAROUND: This is a workaround for the Task Manager issue: https://github.com/elastic/kibana/issues/75501
+      // and should be removed as soon as this issue is resolved.
+      this.logger.error(
+        'Session index cleanup task has been already scheduled, but is missing in the task list for some reason. Please restart Kibana to automatically reschedule this task.'
+      );
+      return;
     }
 
     try {
@@ -151,10 +164,11 @@ export class SessionManagementService {
         state: {},
       });
     } catch (err) {
-      this.logger.error(`Failed to register session index cleanup task: ${err.message}`);
+      this.logger.error(`Failed to schedule session index cleanup task: ${err.message}`);
       throw err;
     }
 
+    this.isCleanupTaskScheduled = true;
     this.logger.debug('Successfully scheduled session index cleanup task.');
   }
 }