Getting Duplicate numbers and gaps in the sequence order

Same problem here. I have no clue what causes this. I think the best way would be to ensure technically that there can not be the same value twice. Perhaps using the database backend (auto_increment in mysql) or something like that which is thread-safe?

Same problem here with 7.x-1.4

I also have this issue with duplicated numbers once a month.

I too have this issue (gaps in the numbering sequence) in Percona Server 5.5.24-55 but in MySQL server 5.5.44 every work fine.

I got this issue on MySQL 5.5 several times. I think it's like race conditions, when two users submit node add form at same time...

I search and here is the problem, wait..., not is a problem...for they.

https://docs.acquia.com/articles/why-do-node-ids-increment-more-one

I have nothing to prove it, but I have the feeling that every time I update serial, the same serial is given twice. See attached images.

Same bug - two nodes assigned same value. Drupal 7.41 (or whatever was current at the end of October 2015), Serial 7.x-1.5. Site runs on Pantheon, so MariaDB. Notably, the two nodes with the same serial id were created more than a week apart (Oct 23, November 6) - this was not a race condition.

Could it have anything to do with the switch to uniqid()? I note the following code in _serial_generate_value(), which does not appear to make any sense. In the old, nid-based code, this same section of code usefully deleted old records. This new code only deletes the most recently created record, and only if the new serial id is divisible by 10. And in my case, the duplicate serial number was indeed divisible by 10. Could this be the source of the bug?

// Delete the temporary record.
  if ($delete && ($sid % 10) == 0) {
    db_delete($table)
      ->condition('uniqid', $uniqid)
      ->execute();
  }

Related: #1905986: Strange Behavior: Missing SIDs in Increments of ten

Attached please find a patch to fix this issue. I note the patches in the related issue, which take a slightly different approach and have failed testing.

It appears that the patch is failing CI testing because the Serial module has no tests.
ERROR: No valid tests were specified.
and
ERROR: Step ?Publish JUnit test result report? failed: None of the test reports contained any result
The failure seems to have nothing to do with the code, or am I missing something?

Looks like a problem with the new testbot. I filed an issue over at #2628260: Testing fails if no tests. I knew there were no tests; I just wanted to make sure the patch applied.

I just disabled the new one & put the old one back on. Let's see if that helps.

I'm having this problem on a new Drupal installation and will test the patch there. Do we have any other reviews yet on patches success to fix the problem?

We're now testing the patch. No problems YET. Any further feedback?

For whatever it's worth, my patch has solved the problem for my client.

// Delete the temporary record.
-  if ($delete && ($sid % 10) == 0) {
+  if ($delete && $sid && ($sid % 10) == 0) {
     db_delete($table)
-      ->condition('uniqid', $uniqid)
+      ->condition('sid', $sid, '<')
       ->execute();
   }

Can you explain what are the rationales behind the old and the new code ? I still don't get the "%10" aspect.

The delete task is slightly expensive. No need to do it more than once every 10 records. But it should only be done if $sid > 0.

The change to the query is simply to make it work - it's the only really important piece. Using the uniquid to find the records to delete just doesn't make sense with the data schema.

Serial 7.x-1.5 - not dev! (db MariaDB).
Today it was the first time in my life (serial-id to duplicate). It's very strange.
May be reason to update the database?

I have noticed the same especially at multiples of 10, I will give the patch at #11 a try and report back.

I've had this issue as well. Going to try the patch.

On investigation, there was a 'Attempting to re-run cron while it is already running.' event.

I wonder if this has anything to do with it.

I've been running #11 for the last 3 months and in that time 20% of my nodes have duplicate serial number numbers... not good!

They all have "created" timestamps that are for the most part the same, and at most 1 second apart.

It is very unlikely that it has to do with DB versions.. Most likely something fails in the module code..

Okay I discovered the cause of my duplicates.

I have a hook_node_insert() that (by design) in some circumstances creates two very similar nodes for every one node a user adds.

I can solve my duplicate issue by either:
1) Unsetting the serial field on the cloned node
or
2) Getting serial field to regenerate when it detects it's a new node

Number 1 would obviously be done in custom code, but 2 might be the safer option to prevent others running into similar issues, it's also a similar approach to that taken over in #2144389: Doesn't work with node_clone, here is one way to implement 2:

 function serial_field_presave($entity_type, $entity, $field, $instance, $langcode, &$items) {
-  if (empty($items)) {
+  if (empty($items) || !empty($entity->is_new)) {
   ...
 }

This change has been made in addition to #11, so leaving as NW in-case someone wishes to roll a combined patch.

It has nothing to do with possibly multiple users adding a new node at the same time. I think the logic used to generate a SID is not working the way it should. Why not simple finding the max current SID and then add to it 1 and only then to save the record?

And a patch.

Given that this bug actually breaks the module, does anyone have any idea why the module was just updated without some kind of related patch? If only to fix it so it doesn't delete the current uniquid, and instead deletes PRIOR records? I understand that MustangGB's most recent patch might be controversial because it combines two different issues into one patch, but can't we at least fix the bog-standard problem that affects everyone? Do we need a new patch?

I agree that my addition could be a separate/follow-up issue if maintainers wish to go that route, of course I'd also be happy to see it squeezed it in here :)

However I should have said that I've been using #11 for nearly 4 months and it does fix the core bug, so I can definitely RTBC based on that.

Hopefully this helps to move the issue forward.

Before we move forward with #11, I'd like to know why it's a better solution than #28. Wrapping that logic (#28) in a transaction seems more understandable to me than what's going on now or with this new patch.

To respond to #31, yes, let's keep other issues in other tickets please. Thanks!

What would that entail?

Not using AUTO_INCREMENT and therefore not using temporary tables at all?

colan, I assume you are referring to MustangGB's #29? If so, the only difference between #11 and #29 is that #29 adds some more code to address an additional, different problem in the module (a problem which can also cause two nodes to appear with the same serial number, but only when a node is copied/cloned).

I have no objection to the addition; it looks great to me, I simply have not tested it. And conceivably there are existing use cases out there where people do want to duplicate a serial number when cloning a node and are depending on the current behavior. I can't imagine why you'd want to do that, but....

As to the technical point, I don't think there's any need to wrap the db_delete in a transaction; you're just doing table cleanup to clear out old records. If it fails, the code will just clean up the next time it is called, and the failure has no effect on the assignment of serial numbers, just on the size of the database. The only thing that does matter is NOT deleting the current record every ten iterations - which is what the un-patched module is doing at the moment.

I hope that helps clarify.

Okay, fair enough. I tried to commit this, but it wouldn't apply. Can I get a reroll of #11 on the latest HEAD? Thanks.

Patch rerolled on HEAD

Thanks. All: Please verify this works with the latest dev branch.

Feel free to open another ticket for the other issue, and provide a link from here.

Linking to the breakaway issue as requested by colan: #2859179: Duplicating a node results in duplicate serial numbers