Skip to content

fix: Crawler quits ChromeDriver on destruction#3070

Merged
ZanSara merged 2 commits intodeepset-ai:mainfrom
danielbichuetti:add_crawler_destructor
Aug 22, 2022
Merged

fix: Crawler quits ChromeDriver on destruction#3070
ZanSara merged 2 commits intodeepset-ai:mainfrom
danielbichuetti:add_crawler_destructor

Conversation

@danielbichuetti
Copy link
Contributor

@danielbichuetti danielbichuetti commented Aug 19, 2022

Related Issues

Proposed Changes:

Today, Crawler will instantiate ChromeDriver, use it, and it will never close the window or the ChromeDriver itself.

This change will close the browser window and quit the ChromeDriver instance during object destruction.

How did you test it?

Tested on all connector node tests.

Notes for the reviewer

After hundreds of Crawler instances being created, there are hundreds of zombie processes left behind. This leads to huge wasted memory being consumed by unused processes.

Checklist

@danielbichuetti danielbichuetti requested review from a team as code owners August 19, 2022 13:11
@danielbichuetti danielbichuetti requested review from vblagoje and removed request for a team August 19, 2022 13:11
@danielbichuetti danielbichuetti changed the title bug: fix Crawler leaving zombie processes after destruction fix: fix Crawler leaving zombie processes after destruction Aug 19, 2022
@danielbichuetti danielbichuetti changed the title fix: fix Crawler leaving zombie processes after destruction fix: Crawler quits ChromeDriver on destruction Aug 19, 2022
Copy link
Contributor

@ZanSara ZanSara left a comment

Choose a reason for hiding this comment

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

Ah good catch! Thank you 😊

@ZanSara ZanSara merged commit 149224f into deepset-ai:main Aug 22, 2022
@danielbichuetti danielbichuetti deleted the add_crawler_destructor branch August 22, 2022 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crawler node doesn't close Webdriver connection

3 participants