Project

Profile

Help

Issue #5668

closed

Using futures in sysncpipeline basically deactivates batching

Added by mdellweg about 5 years ago. Updated over 4 years ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Normal
Assignee:
Category:
-
Sprint/Milestone:
Start date:
Due date:
Estimated time:
Severity:
2. Medium
Version:
Platform Release:
OS:
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
Performance
Sprint:
Sprint 63
Quarter:

Description

Currently, when using await on the result of get_or_create_future of a DeclarativeContent without marking it as unable to batch, will most likely lead to deadlocks, when the future is awaited, while the content is waiting for a batch to be filled.

The related PR: https://github.com/pulp/pulpcore/pull/365
provides the possibility that a DeclarativeContent can be batched and will wait until its associated future is actually awaited. At that very moment it is marked as no longer able to batch up, and it signals the queue it is in to flush the current (incomplete) batch.

This change is completely backwards compatible. It only adds the possibility to use the futures without specifying does_batch=False.

If used reasonably, it can help to increase sync performance in some cases.

Actions #1

Updated by daviddavis about 5 years ago

  • Triaged changed from No to Yes
  • Sprint set to Sprint 61
Actions #2

Updated by dalley about 5 years ago

  • Status changed from NEW to POST
  • Assignee set to mdellweg
Actions #3

Updated by rchan about 5 years ago

  • Sprint changed from Sprint 61 to Sprint 62
Actions #4

Updated by rchan almost 5 years ago

  • Sprint changed from Sprint 62 to Sprint 63

Added by mdellweg almost 5 years ago

Revision 338deb90 | View on GitHub

Add resolution callback to DeclarativeContent

If it is currently waiting in a queue, unfreeze that. Remove old interfaces does_batch and get_or_create_future.

fixes #5668

Actions #5

Updated by mdellweg almost 5 years ago

  • Status changed from POST to MODIFIED
Actions #6

Updated by mdellweg almost 5 years ago

mdellweg wrote:

This change is completely backwards compatible. It only adds the possibility to use the futures without specifying does_batch=False.

This is no longer true. The change removes both does_batch as well as get_or_create_future and adds the resolution coroutine to replace them.

Added by bmbouter almost 5 years ago

Revision d98259db | View on GitHub

Update release notes for Batching change

https://pulp.plan.io/issues/5668 closes #5668

Actions #8

Updated by bmbouter almost 5 years ago

  • Sprint/Milestone set to 3.1.0
Actions #9

Updated by bmbouter almost 5 years ago

  • Status changed from MODIFIED to CLOSED - CURRENTRELEASE
Actions #10

Updated by bmbouter over 4 years ago

  • Tags Performance added
  • Tags deleted (Sync Performance)

Also available in: Atom PDF