From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id 89B70429E5F for ; Tue, 28 Jun 2011 23:37:02 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -0.699 X-Spam-Level: X-Spam-Status: No, score=-0.699 tagged_above=-999 required=5 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id PPHPPIMheAYw for ; Tue, 28 Jun 2011 23:37:01 -0700 (PDT) Received: from mail-qy0-f181.google.com (mail-qy0-f181.google.com [209.85.216.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client certificate requested) by olra.theworths.org (Postfix) with ESMTPS id BAED1429E55 for ; Tue, 28 Jun 2011 23:37:01 -0700 (PDT) Received: by qyk9 with SMTP id 9so602678qyk.5 for ; Tue, 28 Jun 2011 23:37:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; bh=/FU4WYtOFn5Z8Cv3EzJR1Kb2vY0jR2vt26zV/cEEZ2s=; b=C8mSQBKBW7owiEA8lGHe5xncUo9jJ4/8YvM8bkAwg4unbwI3lUAD0S82V1DHcs1/Vi 2aFQGG7Sf+YeM22ePu/z8R8qEt08oaZ8UUD9A5sCcXhwx+3UURpNl5rLfhijFRTnifmJ YC+wFF7whot+IryT2V9it8N+8oa0N2LWv1Vuk= MIME-Version: 1.0 Received: by 10.229.106.32 with SMTP id v32mr299999qco.77.1309329420907; Tue, 28 Jun 2011 23:37:00 -0700 (PDT) Sender: amdragon@gmail.com Received: by 10.229.249.193 with HTTP; Tue, 28 Jun 2011 23:37:00 -0700 (PDT) In-Reply-To: References: <87ei34rnc5.fsf@yoom.home.cworth.org> <1307822683-848-1-git-send-email-amdragon@mit.edu> Date: Wed, 29 Jun 2011 02:37:00 -0400 X-Google-Sender-Auth: Lk8gItFyysmvF-s4Eqe4-U1WkQM Message-ID: Subject: Re: [PATCH v6 00/17] Fix 'notmuch new' atomicity issues From: Austin Clements To: Carl Worth Content-Type: text/plain; charset=ISO-8859-1 Cc: notmuch@notmuchmail.org X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Jun 2011 06:37:02 -0000 Just found one more atomicity bug in notmuch-new. I would prefer to not update this patch series yet again, but rather to follow up with a separate fix for this. The full bug is described below, but the gist is that how add_files_recursive computes new_directory from the mtime isn't sound. The simplest fix is to remove the new_directory optimization, but then we wouldn't scan files in inode order. The best fix is probably to add an out-argument to notmuch_database_get_directory that indicates if the directory really was just created in the database (and hence has no files or subdirs). Unfortunately, that requires an API change. If that's undesirable, an alternate would be to record that bit in the notmuch_directory_t and add some notmuch_directory_is_new API that returns it. Thoughts? Bug description: add_files_recursive determines whether a directory is "new" by inspecting the database mtime returned by notmuch_directory_get_mtime. However, if there's an interruption after adding messages/subdirectories from a new directory but before updating the directory's mtime, it will be considered a new directory again on the next run. As a result, on the next run, db_files and db_subdirs will not be loaded from the database (even though they *wouldn't* be NULL). As a result, we'll miss removing messages or entire subdirectories that have been deleted from the "new" directory. (We'll also re-add the messages in this directory to the database rather than skipping them, which will throw off the notmuch new statistics, but that's fine.) This didn't show up in the atomicity test because throwing off anything besides the statistics requires *two* interruptions. In fact, I don't even know how I would write an automated test for this.