Skip to content

Commit 392647f

Browse files
authored
Hybrid Search: Adding fixes for schedulingStopWatch start/stop for a race condition (#46594)
* Adding fixes for stopWatch when running hybrid search queries * Updating changelog
1 parent 625b333 commit 392647f

File tree

2 files changed

+19
-7
lines changed

2 files changed

+19
-7
lines changed

sdk/cosmos/azure-cosmos/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
* Fixed Null Pointer Exception for query when container recreated with same name. - [PR 45930](https://github.com/Azure/azure-sdk-for-java/pull/45930)
1313
* Fixed Null Pointer Exception for readMany when container recreated with same name. - [PR 45930](https://github.com/Azure/azure-sdk-for-java/pull/45930)
1414
* Fixed parameterized query failures for Hybrid Search queries. - [PR 46446](https://github.com/Azure/azure-sdk-for-java/pull/46446)
15-
* Fixed a rare race in parallel Hybrid Search queries by making internal SchedulingStopwatch start/stop atomic and idempotent. - [PR 46485](https://github.com/Azure/azure-sdk-for-java/pull/46485)
15+
* Fixed a rare race in parallel Hybrid Search queries by making internal SchedulingStopwatch start/stop atomic and idempotent. - [PR 46594](https://github.com/Azure/azure-sdk-for-java/pull/46594)
1616
* Fixed the Hybrid Search SchedulingStopWatch to use Cumulative Timing Across all parallel queries. - [PR 46591](https://github.com/Azure/azure-sdk-for-java/pull/46591)
1717
* Fixed Strong Consistency violation when a single replica in a partition returns a 410 `Lease Not Found`. - [PR 46433](https://github.com/Azure/azure-sdk-for-java/pull/46433)
1818

sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/query/metrics/SchedulingStopwatch.java

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,28 +26,34 @@ public SchedulingTimeSpan getElapsedTime() {
2626

2727
/**
2828
* Tells the SchedulingStopwatch know that the process is in a state where it is ready to be worked on,
29-
* which in turn starts the stopwatch for for response time and turnaround time.
29+
* which in turn starts the stopwatch for response time and turnaround time.
3030
*/
3131
public void ready() {
3232
startStopWatch(this.turnaroundTimeStopwatch);
3333
startStopWatch(this.responseTimeStopwatch);
3434
}
3535

3636
public void start() {
37-
if (!this.runTimeStopwatch.isStarted()) {
37+
synchronized (this.runTimeStopwatch) {
38+
if (this.runTimeStopwatch.isStarted()) {
39+
return;
40+
}
3841
if (!this.responded) {
3942
// This is the first time the process got a response, so the response time stopwatch needs to stop.
40-
this.responseTimeStopwatch.stop();
43+
stopStopWatch(this.responseTimeStopwatch);
4144
this.responded = true;
4245
}
4346
this.runTimeStopwatch.reset();
44-
startStopWatch(this.runTimeStopwatch);
47+
this.runTimeStopwatch.start();
4548
}
4649
}
4750

4851
public void stop() {
49-
if (this.runTimeStopwatch.isStarted()) {
50-
stopStopWatch(this.runTimeStopwatch);
52+
synchronized (this.runTimeStopwatch) {
53+
if (!this.runTimeStopwatch.isStarted()) {
54+
return;
55+
}
56+
this.runTimeStopwatch.stop();
5157
this.numPreemptions++;
5258
}
5359
}
@@ -59,12 +65,18 @@ public void terminate() {
5965

6066
private void startStopWatch(StopWatch stopwatch) {
6167
synchronized (stopwatch) {
68+
if (stopwatch.isStarted()) {
69+
return; // idempotent start
70+
}
6271
stopwatch.start();
6372
}
6473
}
6574

6675
private void stopStopWatch(StopWatch stopwatch) {
6776
synchronized (stopwatch) {
77+
if (!stopwatch.isStarted()) {
78+
return; // idempotent stop
79+
}
6880
stopwatch.stop();
6981
}
7082
}

0 commit comments

Comments
 (0)