googleapis / googleapis/google-cloud-python

Race condition in MetricsTracer causes AttributeError: 'NoneType' object has no attribute 'status'

Đang mở
#15,881 6 bình luận 3 reaction 1 người được giao Được @sinhasubham nhận Xem trên GitHub
api: spanner
Ngôn ngữ chính
Python
Star
5.4k
Fork
1.8k
Merge trung bình
2 ngày 23 giờ
Pull request đã merge (30 ngày)
123

Mô tả

#### Environment details

- OS type and version: Linux (Debian 12 on GKE)
- Python version: 3.11.1
- pip version: 24.0
- `google-cloud-spanner` version: 3.61.0

#### Steps to reproduce

1. Use google-cloud-spanner with SQLAlchemy (`sqlalchemy-spanner` 1.17.2) in a multi-threaded environment (e.g., FastAPI with uvicorn workers)
2. Perform concurrent database operations (SELECT queries) under load
3. The error occurs intermittently

#### Code example

The bug is a race condition in `google/cloud/spanner_v1/metrics/metrics_interceptor.py`:

```python
def intercept(self, invoked_method, request_or_iterator, call_details):
# ...
SpannerMetricsTracerFactory.current_metrics_tracer.set_method(method_name)
SpannerMetricsTracerFactory.current_metrics_tracer.record_attempt_start()
response = invoked_method(request_or_iterator, call_details)
SpannerMetricsTracerFactory.current_metrics_tracer.record_attempt_completion() # BUG: current_metrics_tracer may have been replaced by another thread
# ...
```

`SpannerMetricsTracerFactory.current_metrics_tracer` is a **class variable** shared across all threads. Between `record_attempt_start()` and `record_attempt_completion()`, another thread can call `MetricsCapture.__enter__()` which replaces `current_metrics_tracer` with a new instance:

```python
# In metrics_capture.py __enter__():
SpannerMetricsTracerFactory.current_metrics_tracer = factory.create_metrics_tracer()
```

When `record_attempt_completion()` is called, it operates on a different `MetricsTracer` instance that hasn't had `record_attempt_start()` called, so `current_op.current_attempt` is `None`.

**Suggested fix:** Make `current_metrics_tracer` thread-local instead of a class variable:

```python
import threading

class SpannerMetricsTracerFactory(MetricsTracerFactory):
_metrics_tracer_factory: "SpannerMetricsTracerFactory" = None
_thread_local = threading.local()

@property
def current_metrics_tracer(cls):
return getattr(cls._thread_local, 'metrics_tracer', None)

@current_metrics_tracer.setter
def current_metrics_tracer(cls, value):
cls._thread_local.metrics_tracer = value
```

**Additional bug:** There's a typo in `spanner_metrics_tracer_factory.py` line 93:
```python
cls._metrics_tracer_factory.enabeld = enabled # Should be "enabled" not "enabeld"
```

#### Stack trace
```
File "/usr/local/lib/python3.11/site-packages/google/cloud/spanner_v1/metrics/metrics_interceptor.py", line 148, in intercept
SpannerMetricsTracerFactory.current_metrics_tracer.record_attempt_completion()

File "/usr/local/lib/python3.11/site-packages/google/cloud/spanner_v1/metrics/metrics_tracer.py", line 332, in record_attempt_completion
self.current_op.current_attempt.status = status

AttributeError: 'NoneType' object has no attribute 'status'
```

**Workaround:** Disable built-in metrics by calling `SpannerMetricsTracerFactory(enabled=False)` before any Spanner operations.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.