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

Fix/dual legs transcribe race condition #593

Merged
merged 5 commits into from
Jan 7, 2024
Merged

Conversation

xquanluu
Copy link
Contributor

@xquanluu xquanluu commented Jan 6, 2024

No description provided.

@@ -263,7 +264,8 @@ class TaskTranscribe extends SttTask {
throw new Error(`Invalid vendor ${this.vendor}`);
}
}

// save dedicated bugname for each endpoint
ep.transcribe_bugname = `${bugname}_${Date.now()}`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use a simple integer counter rather than Date.now() ? It might be easier to debug if necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No We can't, due to issue that new transcribe verb can have the same name again to the old transcribe verb is being killed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused though...seems like did make this change below?

@@ -265,7 +266,8 @@ class TaskTranscribe extends SttTask {
}
}
// save dedicated bugname for each endpoint
ep.transcribe_bugname = `${bugname}_${Date.now()}`;
ep.transcribe_bugname = `${bugname}_${channel}_${this.bugname_counter}`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe just

ep.transcribe_bugname = `${bugname}_${channel}_${this.bugname_counter++}`;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we can't use this,

first verb will have transcribe_bugname_1_0
LCC to move the call to new transcribe verb in listen will have same bug name again transcribe_bugname_1_0

in the meanwhile the first transcribe is still being closed, the 2nd transcribe save private data to same bug name again

Still Date.now() is a must to make sure every single time we have different bugname

@davehorton davehorton merged commit 9d70ed9 into main Jan 7, 2024
2 checks passed
@davehorton davehorton deleted the fix/dual_leg_transcribe branch January 7, 2024 00:12
davehorton added a commit that referenced this pull request Jan 9, 2024
davehorton added a commit that referenced this pull request Jan 9, 2024
avoylenko pushed a commit to avoylenko/jambonz-feature-server that referenced this pull request Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants