Skip to content
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

Ensure we do not start a new discovery for an event if one is already scheduled #17339

Merged
merged 2 commits into from
Sep 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/2 Fixes/17339.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Ensure we do not start a new discovery for an event if one is already scheduled.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ export class EnvsCollectionService extends PythonEnvsWatcher<PythonEnvCollection
/** Keeps track of ongoing refreshes for various queries. */
private refreshPromises = new Map<PythonLocatorQuery | undefined, Promise<void>>();

/** Keeps track of whether there are any scheduled refreshes other than the ongoing one for various queries. */
private scheduledRefreshes = new Map<PythonLocatorQuery | undefined, boolean>();

private readonly refreshStarted = new EventEmitter<void>();

public get onRefreshStart(): Event<void> {
Expand All @@ -33,12 +36,18 @@ export class EnvsCollectionService extends PythonEnvsWatcher<PythonEnvCollection

constructor(private readonly cache: IEnvsCollectionCache, private readonly locator: IResolvingLocator) {
super();
this.locator.onChanged((event) =>
this.triggerNewRefresh().then(() => {
this.locator.onChanged((event) => {
const query = undefined; // We can also form a query based on the event, but skip that for simplicity.
const isNewRefreshScheduled = this.scheduledRefreshes.get(query);
if (isNewRefreshScheduled) {
// If there is already a new refresh scheduled for the query, no need to start another one.
return;
}
this.scheduleNewRefresh(query).then(() => {
// Once refresh of cache is complete, notify changes.
this.fire({ type: event.type, searchLocation: event.searchLocation });
}),
);
});
});
this.cache.onChanged((e) => {
this.fire(e);
});
Expand Down Expand Up @@ -71,18 +80,7 @@ export class EnvsCollectionService extends PythonEnvsWatcher<PythonEnvCollection
return refreshPromise;
}

/**
* Ensure we trigger a fresh refresh after the current refresh (if any) is done.
*/
private async triggerNewRefresh(query?: PythonLocatorQuery): Promise<void> {
const refreshPromise = this.getRefreshPromiseForQuery(query);
const nextRefreshPromise = refreshPromise
? refreshPromise.then(() => this.startRefresh(query))
: this.startRefresh(query);
return nextRefreshPromise;
}

private async startRefresh(query: PythonLocatorQuery | undefined): Promise<void> {
private startRefresh(query: PythonLocatorQuery | undefined): Promise<void> {
const stopWatch = new StopWatch();
this.refreshStarted.fire();
const iterator = this.locator.iterEnvs(query);
Expand Down Expand Up @@ -148,4 +146,18 @@ export class EnvsCollectionService extends PythonEnvsWatcher<PythonEnvCollection
// is a superset for every other query, only consider that for simplicity.
return this.refreshPromises.get(query) ?? this.refreshPromises.get(undefined);
}

/**
* Ensure we trigger a fresh refresh for the query after the current refresh (if any) is done.
*/
private async scheduleNewRefresh(query?: PythonLocatorQuery): Promise<void> {
this.scheduledRefreshes.set(query, true);
const refreshPromise = this.getRefreshPromiseForQuery(query) ?? Promise.resolve();
const nextRefreshPromise = refreshPromise.then(() => {
// No more scheduled refreshes for this query as we're about to start the scheduled one.
this.scheduledRefreshes.set(query, false);
this.startRefresh(query);
});
return nextRefreshPromise;
}
}