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

GSK-1863 : Using multiprocess instead of threading #1469

Conversation

Hartorn
Copy link
Member

@Hartorn Hartorn commented Oct 9, 2023

Description

Related Issue

Type of Change

  • 📚 Examples / docs / tutorials / dependencies update
  • 🔧 Bug fix (non-breaking change which fixes an issue)
  • 🥂 Improvement (non-breaking change which improves an existing feature)
  • 🚀 New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to change)
  • 🔐 Security fix

Checklist

  • I've read the CODE_OF_CONDUCT.md document.
  • I've read the CONTRIBUTING.md guide.
  • I've updated the code style using make codestyle.
  • I've written tests for all new methods and classes that I created.
  • I've written the docstring in Google format for all the methods and classes that I used.

@Hartorn Hartorn requested a review from andreybavt October 9, 2023 16:21
@Hartorn Hartorn self-assigned this Oct 9, 2023
@linear
Copy link

linear bot commented Oct 9, 2023

GSK-1863 Using get similar examples on Enron seems to stuck the server/worker ?

Getting the insights on Enron is really slow, but whenclicking on get similar data, it looks like is getting stuck on some computations ?
Actually, even when stopping to debug, and then launching again it seems to stay stuck.

CPU usage is not that high, using only 1 cpu

@Hartorn Hartorn force-pushed the feature/gsk-1863-using-get-similar-examples-on-enron-seems-to-stuck-the branch from 11d7440 to 44191d5 Compare October 9, 2023 22:50
@Hartorn Hartorn changed the title Feature/gsk 1863 using get similar examples on enron seems to stuck the GSK-1863 : Using multiprocess instead of threading Oct 9, 2023
@Hartorn Hartorn marked this pull request as ready for review October 9, 2023 22:55
@Hartorn Hartorn force-pushed the feature/gsk-1863-using-get-similar-examples-on-enron-seems-to-stuck-the branch 5 times, most recently from ac378ad to 8a8edf2 Compare October 10, 2023 09:22
@Hartorn Hartorn requested a review from Inokinoki October 11, 2023 07:42
.gitignore Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
giskard/ml_worker/websocket/listener.py Show resolved Hide resolved
giskard/ml_worker/websocket/__init__.py Outdated Show resolved Hide resolved
# Post-processing of stopWorker
if action == MLWorkerAction.stopWorker and ml_worker.ws_stopping is True:
ml_worker.ws_conn.disconnect()
return


def websocket_actor(action: MLWorkerAction):
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you decide not to add a execute_in_pool boolean flag to this decorator?

Copy link
Member

Choose a reason for hiding this comment

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

Good idea for some quick and short task

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I forgot about that since I had issues with the enron. Just added

logger.debug(f'Registered "{callback.__name__}" for ML Worker "{action.name}"')

def wrapped_callback(ml_worker: MLWorker, req: dict, *args, **kwargs):
dispatch_action(callback, ml_worker, action, req)
Copy link
Contributor

Choose a reason for hiding this comment

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

in case execute_in_pool is added it would need to be passed here

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

@Hartorn Hartorn force-pushed the feature/gsk-1863-using-get-similar-examples-on-enron-seems-to-stuck-the branch from 8a8edf2 to c821df9 Compare October 11, 2023 10:17
@Hartorn Hartorn force-pushed the feature/gsk-1863-using-get-similar-examples-on-enron-seems-to-stuck-the branch from c821df9 to 5dc0701 Compare October 11, 2023 10:20
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Hartorn Hartorn dismissed Inokinoki’s stale review October 12, 2023 08:31

Took into account

@Hartorn Hartorn merged commit bee3029 into main Oct 12, 2023
@Hartorn Hartorn deleted the feature/gsk-1863-using-get-similar-examples-on-enron-seems-to-stuck-the branch October 12, 2023 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants