Optimize IndexEvent retries on the same change
When receiving multiple updates in a very short period of time
for the same change, some of the indexing events will retry
until `index.maxTries` is reached.
This behaviour is resource consuming and adds a warning
in the logs every time an index operation fails
(change/meta SHA1 will never match the one in the event).
To mitigate this situation, this change reindex the change
only if stale and reduce logging level to debug upon retries.
Bug: Issue 320626488
Change-Id: Ia3f96765b36673c9a5e976112e54f73be7208457
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandler.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandler.java
index ac102ff..a787d3a 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandler.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandler.java
@@ -90,13 +90,13 @@
return true;
}
- log.atWarning().log(
+ log.atFine().log(
"Change %s seems too old compared to the event timestamp (event-Ts=%s >> change-Ts=%s)",
id, indexEvent, checker);
return false;
}
- log.atWarning().log(
+ log.atFine().log(
"Change %s not present yet in local Git repository (event=%s)", id, indexEvent);
return false;
@@ -113,7 +113,7 @@
private void reindex(ChangeNotes notes) {
notes.reload();
- indexer.index(notes);
+ indexer.reindexIfStale(notes.getProjectName(), notes.getChangeId());
}
@Override
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandlerTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandlerTest.java
index be74231..27d5b2d 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandlerTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandlerTest.java
@@ -31,6 +31,7 @@
import com.ericsson.gerrit.plugins.highavailability.index.ChangeCheckerImpl;
import com.ericsson.gerrit.plugins.highavailability.index.ForwardedIndexExecutorProvider;
import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.server.index.change.ChangeIndexer;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.util.OneOffRequestContext;
@@ -59,6 +60,7 @@
@Mock private ChangeIndexer indexerMock;
@Mock private ChangeNotes changeNotes;
+ @Mock private Project.NameKey projectName;
@Mock(answer = RETURNS_DEEP_STUBS)
private Configuration configMock;
@@ -87,14 +89,15 @@
public void changeIsIndexedWhenUpToDate() throws Exception {
setupChangeAccessRelatedMocks(CHANGE_EXISTS, CHANGE_UP_TO_DATE);
handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.empty()).get(10, SECONDS);
- verify(indexerMock, times(1)).index(any(ChangeNotes.class));
+ verify(indexerMock, times(1)).reindexIfStale(any(Project.NameKey.class), any(Change.Id.class));
}
@Test
public void changeIsStillIndexedEvenWhenOutdated() throws Exception {
setupChangeAccessRelatedMocks(CHANGE_EXISTS, CHANGE_OUTDATED);
handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.of(new IndexEvent())).get(10, SECONDS);
- verify(indexerMock, atLeast(1)).index(any(ChangeNotes.class));
+ verify(indexerMock, atLeast(1))
+ .reindexIfStale(any(Project.NameKey.class), any(Change.Id.class));
}
@Test
@@ -122,13 +125,13 @@
return null;
})
.when(indexerMock)
- .index(any(ChangeNotes.class));
+ .reindexIfStale(any(Project.NameKey.class), any(Change.Id.class));
assertThat(Context.isForwardedEvent()).isFalse();
handler.index(TEST_CHANGE_ID, Operation.INDEX, Optional.empty()).get(10, SECONDS);
assertThat(Context.isForwardedEvent()).isFalse();
- verify(indexerMock, times(1)).index(any(ChangeNotes.class));
+ verify(indexerMock, times(1)).reindexIfStale(any(Project.NameKey.class), any(Change.Id.class));
}
@Test
@@ -141,7 +144,7 @@
throw new IOException("someMessage");
})
.when(indexerMock)
- .index(any(ChangeNotes.class));
+ .reindexIfStale(any(Project.NameKey.class), any(Change.Id.class));
assertThat(Context.isForwardedEvent()).isFalse();
ExecutionException thrown =
@@ -152,7 +155,7 @@
assertThat(thrown.getCause()).hasMessageThat().isEqualTo("someMessage");
assertThat(Context.isForwardedEvent()).isFalse();
- verify(indexerMock, times(1)).index(any(ChangeNotes.class));
+ verify(indexerMock, times(1)).reindexIfStale(any(Project.NameKey.class), any(Change.Id.class));
}
private void setupChangeAccessRelatedMocks(boolean changeExists, boolean changeIsUpToDate)
@@ -161,7 +164,8 @@
when(changeCheckerFactoryMock.create(TEST_CHANGE_ID)).thenReturn(changeCheckerPresentMock);
when(changeCheckerPresentMock.getChangeNotes()).thenReturn(Optional.of(changeNotes));
}
-
+ when(changeNotes.getChangeId()).thenReturn(id);
+ when(changeNotes.getProjectName()).thenReturn(projectName);
when(changeCheckerPresentMock.isChangeUpToDate(any())).thenReturn(changeIsUpToDate);
}
}
