-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Refactoring GatedAutoCloseable and moving RecoveryState.Timer (#2965)
* Refactoring GatedAutoCloseable to AutoCloseableRefCounted This is a part of the process of merging our feature branch - feature/segment-replication - back into main by re-PRing our changes from the feature branch. GatedAutoCloseable currently wraps a subclass of RefCounted. Segment replication adds another subclass, but this also wraps RefCounted. Both subclasses have the same shutdown hook - decRef. This change makes the superclass less generic to increase code convergence. The breakdown of the plan to merge segment-replication to main is detailed in #2355 Segment replication design proposal - #2229 Signed-off-by: Kartik Ganesh <[email protected]> * Minor refactoring in RecoveryState This change makes two minor updates to RecoveryState - 1. The readRecoveryState API is removed because it can be replaced by an invocation of the constructor 2. The class members of the Timer inner class are changed to private, and accesses are only through the public APIs Signed-off-by: Kartik Ganesh <[email protected]> * Update RecoveryTargetTests to test Timer subclasses deterministically This change removes the use of RandomBoolean in testing the Timer classes and creates a dedicated unit test for each. The common test logic is shared via a private method. Signed-off-by: Kartik Ganesh <[email protected]> * Move the RecoveryState.Timer class to a top-level class This will eventually be reused across both replication use-cases - peer recovery and segment replication. Signed-off-by: Kartik Ganesh <[email protected]> * Further update of timer tests in RecoveryTargetTests Removes a non-deterministic code path around stopping the timer, and avoids assertThat (deprecated) Signed-off-by: Kartik Ganesh <[email protected]> * Rename to ReplicationTimer Signed-off-by: Kartik Ganesh <[email protected]> * Remove RecoveryTargetTests assert on a running timer Trying to serialize and deserialize a running Timer instance, and then checking for equality leads to flaky test failures when the ser/deser takes time. Signed-off-by: Kartik Ganesh <[email protected]>
- Loading branch information
Showing
10 changed files
with
206 additions
and
172 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
97 changes: 97 additions & 0 deletions
97
server/src/main/java/org/opensearch/indices/replication/common/ReplicationTimer.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| /* | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| * The OpenSearch Contributors require contributions made to | ||
| * this file be licensed under the Apache-2.0 license or a | ||
| * compatible open source license. | ||
| */ | ||
|
|
||
| package org.opensearch.indices.replication.common; | ||
|
|
||
| import org.opensearch.common.io.stream.StreamInput; | ||
| import org.opensearch.common.io.stream.StreamOutput; | ||
| import org.opensearch.common.io.stream.Writeable; | ||
| import org.opensearch.common.unit.TimeValue; | ||
|
|
||
| import java.io.IOException; | ||
|
|
||
| /** | ||
| * A serializable timer that is used to measure the time taken for | ||
| * file replication operations like recovery. | ||
| */ | ||
| public class ReplicationTimer implements Writeable { | ||
| private long startTime = 0; | ||
| private long startNanoTime = 0; | ||
| private long time = -1; | ||
| private long stopTime = 0; | ||
|
|
||
| public ReplicationTimer() {} | ||
|
|
||
| public ReplicationTimer(StreamInput in) throws IOException { | ||
| startTime = in.readVLong(); | ||
| startNanoTime = in.readVLong(); | ||
| stopTime = in.readVLong(); | ||
| time = in.readVLong(); | ||
| } | ||
|
|
||
| @Override | ||
| public synchronized void writeTo(StreamOutput out) throws IOException { | ||
| out.writeVLong(startTime); | ||
| out.writeVLong(startNanoTime); | ||
| out.writeVLong(stopTime); | ||
| // write a snapshot of current time, which is not per se the time field | ||
| out.writeVLong(time()); | ||
| } | ||
|
|
||
| public synchronized void start() { | ||
| assert startTime == 0 : "already started"; | ||
| startTime = System.currentTimeMillis(); | ||
| startNanoTime = System.nanoTime(); | ||
| } | ||
|
|
||
| /** | ||
| * Returns start time in millis | ||
| */ | ||
| public synchronized long startTime() { | ||
| return startTime; | ||
| } | ||
|
|
||
| /** | ||
| * Returns elapsed time in millis, or 0 if timer was not started | ||
| */ | ||
| public synchronized long time() { | ||
| if (startNanoTime == 0) { | ||
| return 0; | ||
| } | ||
| if (time >= 0) { | ||
| return time; | ||
| } | ||
| return Math.max(0, TimeValue.nsecToMSec(System.nanoTime() - startNanoTime)); | ||
| } | ||
|
|
||
| /** | ||
| * Returns stop time in millis | ||
| */ | ||
| public synchronized long stopTime() { | ||
| return stopTime; | ||
| } | ||
|
|
||
| public synchronized void stop() { | ||
| assert stopTime == 0 : "already stopped"; | ||
| stopTime = Math.max(System.currentTimeMillis(), startTime); | ||
| time = TimeValue.nsecToMSec(System.nanoTime() - startNanoTime); | ||
| assert time >= 0; | ||
| } | ||
|
|
||
| public synchronized void reset() { | ||
| startTime = 0; | ||
| startNanoTime = 0; | ||
| time = -1; | ||
| stopTime = 0; | ||
| } | ||
|
|
||
| // only used in tests | ||
| public long getStartNanoTime() { | ||
| return startNanoTime; | ||
| } | ||
| } |
Oops, something went wrong.