Skip to content
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

Stop hammering ir_translation id sequence #11

Open
wants to merge 1 commit into
base: 8.0
Choose a base branch
from

Conversation

gracinet
Copy link

@gracinet gracinet commented Aug 5, 2016

Description of the issue/feature this PR addresses: startup failures due to ir_translation_id_seq overflow (DataError)

Current behavior before PR: ir_translation_id_seq grows by big steps, can reach 2**31 in months

Desired behavior after PR is merged: ir_translation_id_seq grows only if new translations appear

(I have NOT signed Odoo CLA)

We've had some instances actually reaching the 2**31 limit, because
the temporary table used to prepare potential insertions used the same
id sequence as the main table, hence each insert in the temporary table
bumps the sequence.

Aggravations:

  • if one hits the wall, the instance won't start
  • if one resets the sequence too close from an existing id, it won't start either
  • the existing ids are spread across the whole spectrum, making it hard to find
    a safe range to reset the sequence.

Yes this probably means instances that restart too much, but that's another issue.

The whole system is questionable at best [1] but this fix should have no impact, and
improve a bit startup performance and PostgreSQL health. On the other hand, we can't
guarantee without a long analysis that's out of our scope that even changing
ir_translation to a bigint would have no impact, yet alone give this the full rewrite
it certainly deserves.

[1] for starters, why use the DB at all in preparation, and use a temporary table
that's logically tied to a specific connection ?

We've had some instances actually reaching the 2**31 limit, because
the temporary table used to prepare potential insertions used the same
id sequence as the main table, hence each insert in the temporary table
bumps the sequence.

Aggravations:

- if one hits the wall, the instance won't start
- if one resets the sequence too close from an existing id, it won't start either
- the existing ids are spread across the whole spectrum, making it hard to find
  a safe range to reset the sequence.

Yes this probably means instances that restart too much, but that's another issue.

The whole system is questionable at best [1] but this fix should have no impact, and
improve a bit startup performance and PostgreSQL health. On the other hand, we can't
guarantee without a long analysis that's out of our scope that even changing
ir_translation to a bigint would have no impact, yet alone give this the full rewrite
it certainly deserves.

[1] for starters, why use the DB at all in preparation, and use a temporary table
    that's logically tied to a specific connection ?
@ccomb
Copy link
Member

ccomb commented Aug 5, 2016

Looks nice. But could you explain a way to reproduce the issue and see the id_seq grow? Could you also point to the version on which this issue was seen? (oca? odoo? branch? commit?)

@gracinet
Copy link
Author

gracinet commented Aug 5, 2016

Target version: the one I'm doing the PR against !
To reproduce, just start Odoo on a database that's already initialized and compare growth of the sequence on one hand, and the highest value that's been actually used in a psql console.
(be sure to avoid developer tricks or modules that precisely avoid reloading PO files)

In my case:

mybase => select nextval('ir_translation_id_seq'); select max(id) from ir_translation;
 nextval 
---------
   63407
(1 row)

  max  
-------
 63362

Then start odoo (and stop it if you like):

bin/start_odoo -d mybase --stop-after-init

And check the values again:

mybase => select nextval('ir_translation_id_seq'); select max(id) from ir_translation;
 nextval 
---------
   67703
(1 row)

  max  
-------
 63362

Conclusion: no actual value has been used, but the sequence has been updated more than 300 times. The more addons you have, the worst it is.

After the fix, you'd see a delta of 1 for nextval (due to the measurement itself)

@gracinet
Copy link
Author

gracinet commented Aug 5, 2016

I've been a bit imprecise in my wording : it occurred in production on a tagged version of that branch but it affects most probably all versions since 2011. We can check that afterwards if you like

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants