Skip to content

Commit 8e826f0

Browse files
fix(firestore): BulkWriter pop from an empty deque (#17490)
In some situations, the BulkWriter would attempt to pop a retry config from an empty deque. This PR adds a safeguard to end immediately if the list is empty Fixes #16138 --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent 53b48ae commit 8e826f0

2 files changed

Lines changed: 42 additions & 0 deletions

File tree

packages/google-cloud-firestore/google/cloud/firestore_v1/bulk_writer.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,8 @@ def _schedule_ready_retries(self) -> None:
508508
)
509509

510510
for _ in range(take_until_index):
511+
if not self._retries:
512+
break
511513
retry: OperationRetry = self._retries.popleft()
512514
retry.retry(self)
513515
return None

packages/google-cloud-firestore/tests/unit/v1/test_bulk_writer.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -759,6 +759,46 @@ def test_basebulkwriter_update_raises_with_bad_option(self):
759759
option=ExistsOption(exists=True),
760760
)
761761

762+
def test_basebulkwriter_reentrancy_empty_deque_retry(self):
763+
# See https://github.com/googleapis/google-cloud-python/issues/16138
764+
import bisect
765+
import datetime
766+
767+
from google.cloud.firestore_v1.bulk_writer import (
768+
BulkWriterCreateOperation,
769+
OperationRetry,
770+
)
771+
772+
client = self._make_client()
773+
bw = _make_no_send_bulk_writer(client)
774+
775+
ref1 = _get_document_reference(client, id="doc1")
776+
ref2 = _get_document_reference(client, id="doc2")
777+
data = {"field": "value"}
778+
779+
# Put ref1 in current operations to trigger a document path collision
780+
# and subsequent flush/enqueue when we create ref1 again.
781+
bw.create(ref1, data)
782+
783+
run_at = datetime.datetime.now(tz=datetime.timezone.utc) - datetime.timedelta(
784+
seconds=1
785+
)
786+
787+
retry1 = OperationRetry(
788+
operation=BulkWriterCreateOperation(reference=ref1, document_data=data),
789+
run_at=run_at,
790+
)
791+
retry2 = OperationRetry(
792+
operation=BulkWriterCreateOperation(reference=ref2, document_data=data),
793+
run_at=run_at,
794+
)
795+
796+
bisect.insort(bw._retries, retry1)
797+
bisect.insort(bw._retries, retry2)
798+
799+
# This should not raise IndexError: pop from an empty deque due to re-entrant scheduling
800+
bw._schedule_ready_retries()
801+
762802

763803
class TestSyncBulkWriter(_SyncClientMixin, _BaseBulkWriterTests):
764804
"""All BulkWriters are opaquely async, but this one simulates a BulkWriter

0 commit comments

Comments
 (0)