Skip to content

Conversation

@JosephCatrambone
Copy link
Contributor

@JosephCatrambone JosephCatrambone commented Jul 9, 2024

Not a real fix as far as I'm concerned, but perhaps the only one we can get.

Before:

% opentelemetry-instrument gunicorn --bind 0.0.0.0:8000 --timeout=30 --threads=2 --workers=2 'guardrails_api.app:create_app("sample.env", "sample_config.py")'
% ab -p ab-post-body.txt -T application/json -s 120 -c 2 -n 100 http://127.0.0.1:8000/guards/name-case/validate
Benchmarking 127.0.0.1 (be patient).....done

Server Software:        gunicorn
Server Hostname:        127.0.0.1
Server Port:            8000

Document Path:          /guards/name-case/validate
Document Length:        103 bytes

Concurrency Level:      2
Time taken for tests:   1.981 seconds
Complete requests:      100
Failed requests:        12
   (Connect: 0, Receive: 0, Length: 12, Exceptions: 0)
Non-2xx responses:      12

After:

opentelemetry-instrument gunicorn --bind 0.0.0.0:8000 --timeout=30 --workers=2 --threads=2 'guardrails_api.app:create_app("sample.env", "sample_config.py")'
% ab -p ab-post-body.txt -T application/json -s 120 -c 2 -n 100 http://127.0.0.1:8000/guards/name-case/validate
Benchmarking 127.0.0.1 (be patient).....done

Server Software:        gunicorn
Server Hostname:        127.0.0.1
Server Port:            8000

Document Path:          /guards/name-case/validate
Document Length:        103 bytes

Concurrency Level:      2
Time taken for tests:   1.970 seconds
Complete requests:      100
Failed requests:        0

Way Too Much Information:

When running concurrently with multiple threads in guardrails-api, there seems to be a concurrent modification of history across two threads. Evidence: Some of the validation_outputs are becoming None between steps of the stack trace. This scales linearly with the number of threads. The same behavior for the same code does not appear if num-threads is set to 1 and the number of workers is increased. (gunicorn --bind 0.0.0.0:8000 --timeout=30 --workers=2 --threads=1 'guardrails_api.app:create_app("sample.env", "sample_config.py")')

A Better Option:

Since we want Guards to be able to be used seamlessly across threads, we should make it possible to modify history and use it in a way that's thread-safe. We could do this by protecting the history object with a mutex or some other kind of thread-safe structure. Unfortunately, the base class is autogenerated and while it's possible that the modification to the subclass will remain compatible, if another type of Guard decides to inherit the interface guard instead of our base guard then we might end up in a place with really troublesome and subtle errors. (Also, we don't want to have to worry about the ordering of the parent classes, though it does potentially make our lives easier: https://docs.python.org/3/tutorial/classes.html#multiple-inheritance)

…d context. Not a real fix as far as I'm concerned.
@JosephCatrambone JosephCatrambone marked this pull request as ready for review July 9, 2024 22:49
@JosephCatrambone JosephCatrambone merged commit feeb138 into 0.5.0-dev Jul 9, 2024
@JosephCatrambone JosephCatrambone deleted the jc/bug_bandaid_null_validation_result branch July 9, 2024 22:57
@notion-workspace
Copy link

Multi-threading issue

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