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 multiple threads share one logger #720

Merged
merged 3 commits into from
Nov 24, 2024
Merged

Conversation

maoyixie
Copy link
Collaborator

The original code (_fuzzing_pipelines, _fuzzing_pipeline, get_trial_logger) will cause multiple threads in the thread pool to share a logger. Now use threading.local() to create a separate logger for each thread.

@DonggeLiu DonggeLiu marked this pull request as draft November 22, 2024 02:28
@DonggeLiu DonggeLiu marked this pull request as ready for review November 22, 2024 02:28
@DonggeLiu
Copy link
Collaborator

Wow, thanks @maoyixie!
Not sure how I missed this, please always feel free to ping me : )

@DonggeLiu
Copy link
Collaborator

Unfortunately this seems to have the same problem:

# Running 3 trials.
2024-11-22 13:33:00 [Trial ID: 01] DEBUG [pipeline.execute]: Pipeline starts
2024-11-22 13:33:00 [Trial ID: 01] DEBUG [pipeline.execute]: Pipeline starts
2024-11-22 13:33:00 [Trial ID: 01] DEBUG [pipeline.execute]: Pipeline starts

Does it work on your local machine?

@maoyixie
Copy link
Collaborator Author

Sorry for the mistake. I have refixed and tested it on the local machine.

@maoyixie maoyixie requested a review from DonggeLiu November 22, 2024 10:47
@maoyixie maoyixie added the bug Something isn't working label Nov 22, 2024
@DonggeLiu
Copy link
Collaborator

DonggeLiu commented Nov 23, 2024

Thanks for fixing this!
Local exp works perfectly now, let me run a final cloud exp to double-check everything if fine too.

@DonggeLiu
Copy link
Collaborator

/gcbrun exp -n my -ag

@DonggeLiu
Copy link
Collaborator

DonggeLiu commented Nov 24, 2024

Report looking good:
https://llm-exp.oss-fuzz.com/Result-reports/ofg-pr/2024-11-24-720-my-comparison/index.html

Log shows different trial numbers too:
image

@DonggeLiu DonggeLiu merged commit a1e2fc0 into google:main Nov 24, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants