Add a Places maintenance task to clean up duplicate URLs

RESOLVED FIXED in Firefox 63

Status

()

enhancement
P2
normal
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: lina, Assigned: lina)

Tracking

(Blocks 1 bug)

unspecified
mozilla63
Points:
---

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

9 months ago
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?
(Assignee)

Comment 2

9 months ago
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)

Updated

9 months ago
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+

Comment 6

9 months ago
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)

Comment 8

9 months ago
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

Comment 9

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/baff5ef35b0d
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months 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.