1478265 - Add a Places maintenance task to clean up duplicate URLs

Status: RESOLVED FIXED

(Toolkit :: Places, enhancement, P2)

Description

[Lina Butler [:lina]]

Bug 1472435 has an interesting case where `moz_places` has two different rows (and different GUIDs) for the same URL and hash. We have a debug-only assert to ensure URLs are unique, and I wonder if we should add a maintenance task to clean them up, too.

I poked at this on the train a bit, and came up with something that might work...consolidating foreign key references for the dupe URLs is the most tedious part. We could simplify by dropping those dupes...or we might decide that the dupes aren't hurting anything. Queries that join on `id` will still return correct results; the mirror ran into this because it joined on URL and hash instead.

WDYT, Mak?

Comment 1

[Lina Butler [:lina]]

Attachment: Add a Places maintenance task to clean up duplicate URLs.

Comment 2

[Lina Butler [:lina]]

Here's what I have so far. I'll add tests and ask for review if you think it's worth doing.

Comment 3

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

I wonder how that happened.

How complicate would be to merge instead of dropping... it would require changing fk and place_id columns around and then removing the orphan entry. Or maybe we should just remove the one that has a 0 foreign_count (provided one does).

Comment 4

[Lina Butler [:lina]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b02b4dc9ed904937d6c15e7343548f1a206bafa4

Comment 5

[Phabricator Automation]

Comment on attachment 8994755 [details]
Add a Places maintenance task to clean up duplicate URLs.

Marco Bonardo [::mak] has approved the revision.

https://phabricator.services.mozilla.com/D2351

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/150241c71d9d
Add a Places maintenance task to clean up duplicate URLs. r=mak

Comment 7

[Cristina Coroiu [:ccoroiu]]

Backed out changeset 150241c71d9d (bug 1478265) for ESlint failure at toolkit/components/places/tests/maintenance/test_preventive_maintenance.js.

Backout: https://hg.mozilla.org/integration/autoland/rev/0348d472115d10b72db677eaea94d0b0e2c67081

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=150241c71d9d812a8bd96b59043421441ff3897f

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=191298403&repo=autoland&lineNumber=266

[task 2018-08-01T05:08:43.614Z] creating build/temp.linux-x86_64-2.7/psutil
[task 2018-08-01T05:08:43.614Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2018-08-01T05:08:43.614Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2018-08-01T05:08:43.614Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_linux.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o
[task 2018-08-01T05:08:43.614Z] creating build/lib.linux-x86_64-2.7
[task 2018-08-01T05:08:43.614Z] creating build/lib.linux-x86_64-2.7/psutil
[task 2018-08-01T05:08:43.614Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so
[task 2018-08-01T05:08:43.614Z] building 'psutil._psutil_posix' extension
[task 2018-08-01T05:08:43.614Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2018-08-01T05:08:43.614Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2018-08-01T05:08:43.614Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so
[task 2018-08-01T05:08:43.614Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so -> psutil
[task 2018-08-01T05:08:43.614Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so -> psutil
[task 2018-08-01T05:08:43.614Z] 
[task 2018-08-01T05:08:43.614Z] Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
[task 2018-08-01T05:14:04.763Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/toolkit/components/places/tests/maintenance/test_preventive_maintenance.js:1285:25 | 'type' is assigned a value but never used. (no-unused-vars)
[taskcluster 2018-08-01 05:14:05.251Z] === Task Finished ===
[taskcluster 2018-08-01 05:14:05.252Z] Unsuccessful task run with exit code: 1 completed in 585.471 seconds

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/baff5ef35b0d
Add a Places maintenance task to clean up duplicate URLs. r=mak

Comment 9

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/baff5ef35b0d

Comment 10

[Mark Banner (:standard8)]

This was relanded so NI is obsolete.