← Back to team overview

launchpad-dev team mailing list archive

Re: verifying fix of attempt to write to read-only slave

 



On Sat, Jan 16, 2010 at 12:57 AM, Edwin Grubbs <edwin.grubbs@xxxxxxxxxxxxx> wrote:
On Fri, Jan 15, 2010 at 1:46 AM, Stuart Bishop
<stuart.bishop@xxxxxxxxxxxxx> wrote:


On Fri, Jan 15, 2010 at 1:48 PM, Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>
wrote:

On Fri, 2010-01-15 at 13:02 +0700, Stuart Bishop wrote:

On Fri, Jan 15, 2010 at 12:00 PM, Edwin Grubbs
<edwin.grubbs@xxxxxxxxxxxxx> wrote:
> I can't figure out how to test bug 506963 on launchpad.dev. This is in
> part because I can't understand how this error could have possibly
> happen. The form is submitted as a POST, so the dbpolicy should choose
> the master main store. Although the EmailAddress is normally retrieved
> from the auth store, which appears to replicate that table from the
> main store, the place where this error occurs should be getting the
> email address from the store provided by Store.of(person).
>
> Does anyone know what is going on here?

You have things backwards. EmailAddress is part of the auth store, and
available read-only via the main store. Store.of(person) will
generally give you the main store.

Best way of fixing this sort of thing is if you need to write to an
object, cast it to the master version - IMasterObject(emailaddress)
will be a no-op if you already have the writable object and is future
proof when tables get shuffled around.

lib/canonical/launchpad/doc/storm.txt for more details

This is the context of the bug that Edwin is fixing.

  3 InternalError: Slony-I: Table emailaddress is replicated and cannot
be modified on a subscriber node
   Bug: https://launchpad.net/bugs/464161
   POST: 3  Robots: 0  Local: 3
      3 https://launchpad.net/%
7Ekorhelvtak/+deactivate-account (Person:+deactivate-account)
       OOPS-1475C322, OOPS-1475E193, OOPS-1475H234

From https://lp-oops.canonical.com/oops.py/?oopsid=OOPS-1475H234, SQL
statement #30 shows attempting to update EmailAddress using the wrong store.
Unfortunately the traceback doesn't show the code that actually did this -
the traceback is raised later when Storm attempts to flush the database
changes.

The real hard part was figuring why our tests for deactivateAccount()
weren't breaking. Though launchpad.dev doesn't use different dbs, it
could use different users for the auth store and the main store, and
after investigating it turns out it does. Then, I found a very
non-obvious permission problem. Though, launchpad_main only has read
access to the EmailAddress table, it is also a member of the pg group
"write", which does have access to insert or update but not delete
from that table. Removing launchpad_main from "write" caused other
things to blow up, since launchpad_main doesn't have explicit access
to the Person table. Revoking insert/append access from "write"
allowed me to recreate the error and verify that IMasterObject() does
indeed fix it.

I'm not sure what the correct change to the permissions is for the
test databases, so that we catch these problems earlier next time.
Should launchpad_main get explicit access to all the tables it needs
so that it can be removed from the "write" group?

The write group should not have write permissions to tables not in the lpmain replication set. I agree that this is a non-obvious permission bug.

I fully support all changes that reduce our dependence on the write and read groups in security.cfg. One day I'll assemble the tools needed to properly audit our permission (record what the test suite does, what production does, compare with what permissions are granted). One big problem is many of our tests rely on overgenerous permissions, creating and modifying rows in the database when authenticated as users that really shouldn't need those permissions.

If tests pass with write access to the EmailAddress table removed from the [write] group, please land that. I'm not sure if copying all the permissions from the [write] group to other users such as launchpad_main, revoking their membership in this group, helps much. If you think it does then go for it.

Thankfully, these issues generally only occur on the border. So Person, EmailAddress, Account & MailingListSubscription are where problems like this are most likely to appear. Also, thankfully, we should be able to drop the authdb replication set entirely in a month or three when login.launchpad.net is replaced by login.ubuntu.com bringing some sanity back to the situation.

--
Stuart Bishop <stuart@xxxxxxxxxxxxxxxx>
http://www.stuartbishop.net/

Attachment: signature.asc
Description: OpenPGP digital signature


References