Skip to content

Conversation

neunhoef
Copy link
Member

@neunhoef neunhoef commented Aug 18, 2025

This addresses https://arangodb.atlassian.net/browse/BTS-2209

The problem arises if there is an AQL INSERT statement in a loop
for a collection which has either exactly one shard or it has
multiple shards and the option forceOneShardAttributeValue is
used for the case that no user-specified _key is given.

Design doc: https://github.com/arangodb/documents/blob/master/DesignDocuments/03_IN_PROGRESS/DuplicateIdsFix.md

Scope & Purpose

  • [*] 💩 Bugfix
  • [*] 🍕 New feature

Checklist

@neunhoef neunhoef added this to the devel milestone Aug 18, 2025
@neunhoef neunhoef self-assigned this Aug 18, 2025
@cla-bot cla-bot bot added the cla-signed label Aug 18, 2025
@neunhoef neunhoef marked this pull request as ready for review August 19, 2025 13:21
@@ -1138,31 +1138,17 @@ function KeyGenerationLocationSuite() {
db._drop(cn);
},

testThatFailurePointsWork1: function () {
Copy link
Member Author

Choose a reason for hiding this comment

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

This test has never worked, since if the failure point did not work, it was green. Now, it cannot be kept, since there should be no situation any more which triggers the single server key generator on a dbserver in a cluster.

try {
c.insert({});
fail();
Copy link
Member Author

Choose a reason for hiding this comment

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

This is necessary to make the test fail, if the failure point does not work.

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.

Technically good.

I just have some minor requests on documentation and tests.

@dothebart
Copy link
Contributor

should there as well be test covearge from this in the dump/restore tests in:

  • tests/js/client/dump/dump-setup-common.inc
  • tests/js/client/dump/dump-test.inc
    ?

@neunhoef
Copy link
Member Author

No, I do not think we need test coverage in dump/restore for this. In the end, restore operations will usually include keys, so key generation will not be triggered at all, as far as I can see.

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

@neunhoef neunhoef merged commit 78b1233 into devel Aug 26, 2025
5 checks passed
@neunhoef neunhoef deleted the bug-fix/unique-ids-oneshard branch August 26, 2025 14:07
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.

3 participants