8000 [3.8] make AQL modification operations async by jsteemann · Pull Request #14525 · arangodb/arangodb · GitHub
[go: up one dir, main page]

Skip to content

[3.8] make AQL modification operations async #14525

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

Closed
wants to merge 35 commits into from

Conversation

jsteemann
Copy link
Contributor
@jsteemann jsteemann commented Jul 19, 2021

Will not be merged to 3.8, as the much simpler bugfix #14695 (respectively #14699) suffices to resolve deadlocks.

Scope & Purpose

This PR makes the AQL modification operations INSERT, UPDATE, REPLACE, REMOVE return futures so they become non-blocking. UPSERT is currently still blocking, but this will be addressed soon.

UPSERT needs to be made async later.

  • 💩 Bugfix (requires CHANGELOG entry)
  • 🍕 New feature (requires CHANGELOG entry, feature documentation and release notes)
  • 🔥 Performance improvement
  • 🔨 Refactoring/simplification
  • 📖 CHANGELOG entry made

Backports:

  • No backports required

Testing & Verification

  • This change is a trivial rework / code cleanup without any test coverage.
  • The behavior in this PR was manually tested
  • This change is already covered by existing tests, such as (please describe tests).
  • This PR adds tests that were used to verify all changes:
    • Added new C++ Unit tests
    • Added new integration tests (e.g. in shell_server / shell_server_aql)
    • Added new resilience tests (only if the feature is impacted by failovers)
  • There are tests in an external testing repository:
  • I ensured this code runs with ASan / TSan or other static verification tools

This PR makes the AQL modification operations INSERT, UPDATE, REPLACE,
REMOVE return futures so they become non-blocking. UPSERT is currently
still blocking, but this will be addressed soon.

Async operations work for `produceRows` already in single server and
cluster mode, but `skipRowsRange` does not yet work in cluster mode.

This still needs to be sorted out. After that, UPSERT needs to be made
async.
@jsteemann jsteemann added this to the 3.8 milestone Jul 19, 2021
@goedderz goedderz requested a review from mchacki August 9, 2021 15:09
@goedderz goedderz self-assigned this Aug 10, 2021
@goedderz
Copy link
Member

Copy link
Member
@mchacki mchacki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking my review into account.
For my eyes this now looks good 👍

@goedderz
Copy link
Member

Copy link
Member
@mchacki mchacki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Also good handling of theoretically impossible code path.

goedderz added a commit that referenced this pull request Aug 16, 2021
@goedderz goedderz changed the title make AQL modification operations async [3.8] make AQL modification operations async Aug 16, 2021
Copy link
Member
@mchacki mchacki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@goedderz
Copy link
Member

As to DO_NOT_MERGE: We most probably found a way to resolve the deadlocks with a much less intrusive patch, which is preferable for 3.8. The changes here are still desirable and already merged to devel, but we might get around without merging them into a stable branch.

@goedderz goedderz closed this Aug 25, 2021
@KVS85 KVS85 deleted the feature-3.8/async-aql-modification-operations branch March 24, 2022 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0