Follow-up to #2664466: url_alias table is only used by a core service but it depends on system install.
We made a mistake making url_alias auto-creatable. This means that until a url_alias is created we throw and exception on many/every requests - see #2696813: url_alias table missing?
Re-add the table to system_schema. Reverting #2664466: url_alias table is only used by a core service but it depends on system install is painful and unnecessarily complex and risky.
None
None
None
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 2704821-5.patch | 1.73 KB | alexpott |
| #5 | 2704821-5.test-only.patch | 982 bytes | alexpott |
| #4 | 2704821-4.patch | 786 bytes | alexpott |
Comments
Comment #2
alexpottComment #3
alexpottComment #4
alexpottHow to test...
Comment #5
alexpottI think given the situation we should consider committing this to 8.1.x-rc.
The test only patch is the interdiff.
Comment #7
alexpottSo yes this is going to use exceptions to communicate on every request because PathProcessorAlias is going to process every inbound path. Discussed this with @xjm and we agreed that fixing this is critical.
Comment #8
alexpottHowever, I'm not sure that the cost of exceptions is that much given this http://zurb.com/forrst/posts/Speed_performance_of_Trying_and_catching_Ex...
Comment #9
berdirLooks good to me.
I haven't tested how slow it actually it, but to be fair, it's not just throwing of a plain exception. It's a pdo exception, that..
1. We catch in \Drupal\Core\Database\Connection::query
2. parse query string and error code in \Drupal\Core\Database\Connection::handleQueryException, re-throwing it
3. catch again in \Drupal\Core\Path\AliasStorage::load
4. do a table exists query in \Drupal\Core\Path\AliasStorage::catchException(), by querying information_schema.tables on mysql.
That can't be very fast :)
Comment #10
catchThe cache backends just eat exceptions on get.
#9-#4 looks like the problematic bit here - we either need to create the table, or not care that it doesn't exist.
To me looks like three options:
1. Stop calling $this->catchExeption() on read.
2. Create the table on read if it's missing
3. the patch here.
To keep things self-contained, I'd prefer #1 or #2 if we can, but I can also see doing #3 then revisiting this in a major for 8.2.x
Comment #11
berdirThe reason we check that is to know if it's a real error or not.
What I discussed with @alexpott in IRC is that we could introduce a special exception class that we'd identifiy based on the error code, we already have one example for that.
Comment #12
catchThere's no standard error code for missing tables, that's been discussed elsewhere and is the reason the cache backends just eat exceptions.
What about creating the table if it's missing on read?
Comment #13
catchLet's do all that in a follow-up, opened one at #2706001: Remove url_alias table from system_install() again.
Definitely an rc target for me - this is a fatal introduced by changes in 8.1.x and the fix here is straightforward.
Comment #16
catchCommitted/pushed to 8.2.x and cherry-picked to 8.1.x. Thanks!