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.

Sign up for GitHub

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

fix(RequestQueueV2): implement an isFinished escape hatch if the queue ends up in what seems to be a deadlock state #2652

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

fix(RequestQueueV2): implement an isFinished escape hatch if the queue ends up in what seems to be a deadlock state #2652

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions packages/core/src/storages/request_provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ export abstract class RequestProvider implements IStorage {

protected isFinishedCalledWhileHeadWasNotEmpty = 0;

protected isFinishedWarningsEmitted = 0;

constructor(
options: InternalRequestProviderOptions,
readonly config = Configuration.getGlobalConfig(),
Expand Down Expand Up @@ -612,6 +614,8 @@ export abstract class RequestProvider implements IStorage {
clientKey: this.clientKey,
},
);

this.isFinishedWarningsEmitted++;
} else {
this.log.debug(
'Queue head still returned requests that need to be processed (or that are locked by other clients)',
Expand All @@ -622,6 +626,12 @@ export abstract class RequestProvider implements IStorage {
}
}

// Temporary deadlock exit: somehow, we can end up in a state where the isEmpty function says its true, but then the head is not only not empty,
// but never empties either! This is a temporary fix to let crawlers exit after a while if they're stuck in this state.
if (this.isFinishedWarningsEmitted >= 3) {
return true;
}

return currentHead.items.length === 0;
}

Expand Down
7 changes: 7 additions & 0 deletions packages/core/src/storages/request_queue_v2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,14 +177,21 @@ export class RequestQueue extends RequestProvider {

// Stop fetching if we are paused for migration
if (this.queuePausedForMigration) {
this.log.debug('Queue is paused for migration, skipping fetching of new requests');
return;
}

// We want to fetch ahead of time to minimize dead time
if (this.queueHeadIds.length() > 1) {
// Not logging here because that'll be _real_ spammy
return;
}

// TODO: if this logs multiple times in a row, we need to double check that we don't need a mutex!
if (!this._listHeadAndLockPromise) {
this.log.debug('Scheduling fetching of new requests from queue head');
}

this._listHeadAndLockPromise ??= this._listHeadAndLock().finally(() => {
this._listHeadAndLockPromise = null;
});
Expand Down