-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Expose Last Index Request Timestamp in Cat Indices API #18405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Expose Last Index Request Timestamp in Cat Indices API #18405
Conversation
…the index Signed-off-by: Sriram Ganesh <[email protected]>
… timestamp Signed-off-by: Sriram Ganesh <[email protected]>
4cffd55
to
3bfecce
Compare
❌ Gradle check result for 3bfecce: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
} | ||
|
||
public Engine.DeleteResult applyDeleteOperationOnReplica(long seqNo, long opPrimaryTerm, long version, String id) throws IOException { | ||
if (indexSettings.isSegRepEnabledOrRemoteNode()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this code removed?
@@ -1103,6 +1110,10 @@ public Engine.IndexResult applyIndexOperationOnPrimary( | |||
sourceToParse, | |||
null | |||
); | |||
if (result.getFailure() == null) { | |||
lastIndexRequestTimestamp = System.currentTimeMillis(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is definitely hot path code, so we want to be careful about adding things like System.currentTimeMillis()
unconditionally to every request. We do have ThreadPool::absoluteTimeMillis
which is probably better to use here. It reduces the resolution of the time value returned but it is probably good enough for this use case.
} | ||
|
||
@Override | ||
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this added?
@@ -95,6 +104,9 @@ public ShardStats(StreamInput in) throws IOException { | |||
if (in.getVersion().onOrAfter(Version.V_3_0_0)) { | |||
pollingIngestStats = in.readOptionalWriteable(PollingIngestStats::new); | |||
} | |||
if (in.getVersion().onOrAfter(Version.V_3_0_0)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of the checks here should be on or after 3.1, not 3.0
assertTrue("Timestamp should be set after first write", ts1 > 0); | ||
|
||
// Wait and index another document | ||
Thread.sleep(1000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed sleeps are almost always bad. In this case it makes the test slower than it needs to be, and technically there's no guarantee that the underlying system clock will actually change within 1 second (though that's unlikely to be a problem in practice here). You can use the assertBusy(() -> ...)
helper to execute the following code in a loop with back off and retry. It will almost always pass on the first iteration (much faster than 1 second), but in case the test machine clock is behaving weird it will still work as expected.
client().prepareIndex("test").setId("2").setSource("field", "value2").setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).get(); | ||
statsResponse = client().admin().indices().prepareStats("test").get(); | ||
long ts2 = statsResponse.getIndex("test").getLastIndexRequestTimestamp(); | ||
assertTrue("Timestamp should increase after another write", ts2 > ts1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use matchers that will give good error messages by default:
assertTrue("Timestamp should increase after another write", ts2 > ts1); | |
assertThat(ts2, greaterThan(ts1)); |
Description
This PR enhances the _cat/indices API by adding two new columns to report the timestamp of the last index request processed for each index:
This mirrors the existing approach for creation.date and creation.date.string, providing both a machine-friendly and a human-friendly representation.
Details
GET _cat/indices?v&h=index,last_index_request_timestamp,last_index_request_timestamp.string
Manual Testing
Related Issues
Resolves #10766
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.