Domain IDs are generated in Drupal\domain\Entity\Domain::createDomainId() using the following code:

$this->domain_id = abs((int) crc32($this->id()));

Example:

php -r 'print abs((int) crc32("some_domain"));'

will return 629525447 and 3665441849 using 32-bit and 64-bit versions of php respectively. Initially importing a new domain record created on a 32-bit system on a 64-bit system will work (the active config gets updated with a new domain id), but subsequent config import operations will fail with a ConfigValueException "The hostname ... is already registered".

CommentFileSizeAuthor
#17 2908236-domain-id-fix.patch2.04 KBagentrickard
#11 2908236-domain-hash.patch746 bytesagentrickard
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Patrick R. created an issue. See original summary.

Patrick R.’s picture

Patrick R. CreditAttribution: Patrick R. at UEBERBIT GmbH commented
Issue summary: View changes
agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented

Any ideas for how to address this issue? To test it?

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented

The big fat warning on this page -- http://php.net/manual/en/function.crc32.php -- is new since this code was written.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented

(I am very tempted to call this non-critical because you should never be importing / exporting of non-compatible systems.)

That rant aside, I am open to solutions. The problem is that we need a numeric id for {node_access}. The other hashing functions all seem to return hex. We can't use hex. We need a consistent decimal.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented

@Patrick R.

Can you test the output of these on the two systems to see if either returns the same value on both?

hash('crc32', 'some_domain')
hexdec(hash('crc32b', 'some_domain'))

Or, really, any other hash that when wrapped in hexdec() will return an integer small enough to fit in the {node_access} table.

Patrick R.’s picture

Patrick R. CreditAttribution: Patrick R. at UEBERBIT GmbH commented

Both of these lines returned the same value on both systems:

hash('crc32', 'some_domain') => e2a02d1f
hexdec(hash('crc32b', 'some_domain')) => 3665441849
agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented

Thanks!

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented

So now we run into the issue of MySQL INT storage.

Max signed INT is 2147483647. Unsigned is 4294967295.

So these functions may returns numbers to high to store. I'd be fine dropping 2 digits here if we have to, as I think the odds of a collision are very, very, small.

The adler32 hash seems shorter, so maybe we use hexdec(hash('adler32', 'some_domain'))

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented

Perhaps I am overthinking. crc32 has worked, so maybe we keep using that, just run through hash().

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented
Status: Active » Needs review
FileSize
2908236-domain-hash.patch746 bytes

Patch attached. Testing running over at https://github.com/agentrickard/domain/pull/363

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented
Status: Needs review » Fixed

Committed.

Patrick R.’s picture

Patrick R. CreditAttribution: Patrick R. at UEBERBIT GmbH commented
Status: Fixed » Needs work

Sorry for the late feedback but unfortunately there are still cases where abs(hexdec(hash('crc32', $this->id()))) will return results greater than PHP_INT_MAX on 32-bit PHP so you should consider reverting the commit. :-( Maybe it would indeed be better to drop digits from the generated IDs if there is no other way around this.

Random thought: maybe drop the numeric domain ID from the Domain entity altogether and use a domain <=> gid mapping stored in key_value table ? That way of course you could end up with different numeric IDs for the same domain on different instances of your site unless you're syncing the database, but that was possible in D7 too, wasn't it ?

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented

We need the value, sadly. I think we can safely drop a few digits without issue. Maybe just divide by 100? Ideally we could test the condition. Do you know what strings generate a number too high?

I don't really want to introduce key-value storage. On D7, we exported the numeric id as part of Features, and hoped things stayed in sync.

Patrick R.’s picture

Patrick R. CreditAttribution: Patrick R. at UEBERBIT GmbH commented

Well, you only need to run a couple of php -r "print abs(hexdec(hash('crc32', '...')));" calls with some arbitrary domains and check if they produce numbers greater than 2147483647. I just made three trials and came up with two hits, drupal_org (2199377096) and whitehouse_gov (3735937102), so it should probably not be too hard to find even more examples. :-)

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented

Thanks. So we can write a test for that.

My inclination, since these numbers are so large, is simply to lop off 2 digits and validate that it doesn't create a duplicate id.

Fortunately, this won't affect existing domains.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented
Status: Needs work » Needs review
FileSize
2908236-domain-id-fix.patch2.04 KB

Here's a solution that should work. The logic is:

* Take the crc32 hash (base 16).
* Drop the last two digits.
* Convert to base10
* Check that the value is unique
* If not unique, increment by 1 until it is unique.

  • agentrickard committed f82cabf on 8.x-1.x 
    Issue #2908236 by agentrickard, Patrick R.: Domain ID may vary between...
agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented

Committed again!

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented
Status: Needs review » Fixed
Patrick R.’s picture

Patrick R. CreditAttribution: Patrick R. at UEBERBIT GmbH commented

Looks good to me. Applied the patches and regenerated the ID from the domain that triggered the whole thing here and it works. Thanks for your effort! :-)

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard at Palantir.net commented

Thanks for running tests that I can't!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.