Closed Bug 1478265 Opened 3 years ago Closed 3 years ago

Add a Places maintenance task to clean up duplicate URLs

Categories

(Toolkit :: Places, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: lina, Assigned: lina)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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?
Here's what I have so far. I'll add tests and ask for review if you think it's worth doing.
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).
Blocks: 1410877
Priority: -- → P2
Assignee: nobody → lina
Status: NEW → ASSIGNED
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
Attachment #8994755 - Flags: review+
Pushed by kcambridge@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/150241c71d9d
Add a Places maintenance task to clean up duplicate URLs. r=mak
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
Flags: needinfo?(lina)
Pushed by kcambridge@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/baff5ef35b0d
Add a Places maintenance task to clean up duplicate URLs. r=mak
https://hg.mozilla.org/mozilla-central/rev/baff5ef35b0d
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
This was relanded so NI is obsolete.
Flags: needinfo?(lina)
You need to log in before you can comment on or make changes to this bug.