Re: need to call notmuch_threads_get (..) to actually move iterator
Gaute Hope writes: > Hi, > > it seems to be necessary to actually call notmuch_threads_get (threads) > to move the thread iterator from a query object, just calling > notmuch_threads_move_to_next (..) is not enough: > I'm not sure of all the details, but it is true that the notmuch_threads_get has side effects in the database. In particular it allocates new thread-ids. So I can easily imagine things failing if you leave out the call. d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v3 16/16] add "notmuch reindex" subcommand
Daniel Kahn Gillmor writes: > This new subcommand takes a set of search terms, and re-indexes the > list of matching messages using the supplied options. > > This can be used to index the cleartext of encrypted messages with > something like: > > notmuch reindex --try-decrypt \ > tag:encrypted and not tag:index-decrypted I haven't reviewed this patch yet. Before I do I'd like to discuss the issue of the apparent existing bug in thread handling that it exposes[1]. Assuming that analysis is correct (and I have no reason not to believe so), the bug is in existing notmuch code and not related to this change. On the other hand, currently users have to work a bit to expose this bug, while this command would be inherently buggy (through no fault of it's own) from introduction. With my release manager hat on, I'm not very happy with the hypothetical announcement "We have this new command, but it will break your threads". So what I'd like to understand is to what extent the reindex command, with the current notmuch codebase (i.e. no new ghost message code), is "suitable for release". One option would be to merge a version of most of the proposed changes (with some minor updates), and leave the re-index (and maybe python bindings changes) for a second series. [1]: id:871t8ko50r@alice.fifthhorseman.net ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v3 15/16] added notmuch_message_reindex
Daniel Kahn Gillmor writes:
> +const char *autotags[] = {
> + "attachment",
> + "encrypted",
> + "signed",
> + "index-decrypted",
> + "index-decryption-failed" };
Hmm. Is this really the only place these values are needed? That's a bit
surprising to me.
> +/* cache tags and filenames */
> +tags = notmuch_message_get_tags(message);
> +filenames = notmuch_message_get_filenames(message);
> +orig_filenames = notmuch_message_get_filenames(message);
> +
> +/* walk through filenames, removing them until the message is gone */
> +for ( ; notmuch_filenames_valid (filenames);
> + notmuch_filenames_move_to_next (filenames)) {
> + filename = notmuch_filenames_get (filenames);
What's the expected lifetime of the tags, filenames, and orig_filenames
lists? I guess they live until message dies. Are we expecting message to
die fairly quickly in the usual case?
> +
> +/* re-add the filenames with the associated indexopts */
> +for (; notmuch_filenames_valid (orig_filenames);
> + notmuch_filenames_move_to_next (orig_filenames)) {
> + filename = notmuch_filenames_get (orig_filenames);
> +
> + status = notmuch_database_add_message_with_indexopts(notmuch,
> + filename,
> + indexopts,
> + readded ? NULL :
> &newmsg);
I guess you thought about this already, but I take it you made an
intentional choice to (attempt to) reindex all the files rather than use
_notmuch_message_add_file_name?
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
Re: need to call notmuch_threads_get (..) to actually move iterator
Really might be the issue: Threads with many messages duplicate more
often than messages with few messages. Threads with only one message
never seem to come up more than twice.
Currently I don't have the time to track down what actually get's stored
in match_set, would be nice if a dev could shed some light on it. Thx!
Excerpts from Franz Fellner's message of Februar 28, 2016 3:26 :
It might be I found the issue:
One big thing notmuch_threads_get does is remove the thread_id from the
match_set. Playing with astroid (branch ti-skip-and-load) i see that it
is not entirely true that the iterator doesn't move. It just seems to
duplicate some messages. There definitely are coming new messages.
So my explanation is that match_set contains duplicate thread_ids. If
that is true a solution to Gautes problem might be to deduplicate the
match_set.
Excerpts from Gaute Hope's message of Februar 24, 2016 1:08 :
Hi,
it seems to be necessary to actually call notmuch_threads_get (threads)
to move the thread iterator from a query object, just calling
notmuch_threads_move_to_next (..) is not enough:
```
notmuch_query_t *query;
notmuch_threads_t *threads;
notmuch_thread_t *thread;
query = notmuch_query_create (database, query_string);
threads = notmuch_query_search_threads (query);
int i = 0;
for (;
notmuch_threads_valid (threads);
notmuch_threads_move_to_next (threads))
{
/*
* with this line commented out the iterator seems to remain in
* place, and if I below do another loop it will start from the
* beginning.
thread = notmuch_threads_get (threads);
notmuch_thread_destroy (thread);
*/
i++;
if (i > 100) break;
}
for (;
notmuch_threads_valid (threads);
notmuch_threads_move_to_next (threads))
{
/* the thread acquired here will be the first thread in the query.
* it should be the 101th. */
thread = notmuch_threads_get (threads);
notmuch_thread_destroy (thread);
}
notmuch_query_destroy (query);
```
It is quite slow to skip the threads in this way, might it be faster if
move_to_next works correctly?
Regards, Gaute
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
Re: need to call notmuch_threads_get (..) to actually move iterator
It might be I found the issue:
One big thing notmuch_threads_get does is remove the thread_id from the
match_set. Playing with astroid (branch ti-skip-and-load) i see that it
is not entirely true that the iterator doesn't move. It just seems to
duplicate some messages. There definitely are coming new messages.
So my explanation is that match_set contains duplicate thread_ids. If
that is true a solution to Gautes problem might be to deduplicate the
match_set.
Excerpts from Gaute Hope's message of Februar 24, 2016 1:08 :
Hi,
it seems to be necessary to actually call notmuch_threads_get (threads)
to move the thread iterator from a query object, just calling
notmuch_threads_move_to_next (..) is not enough:
```
notmuch_query_t *query;
notmuch_threads_t *threads;
notmuch_thread_t *thread;
query = notmuch_query_create (database, query_string);
threads = notmuch_query_search_threads (query);
int i = 0;
for (;
notmuch_threads_valid (threads);
notmuch_threads_move_to_next (threads))
{
/*
* with this line commented out the iterator seems to remain in
* place, and if I below do another loop it will start from the
* beginning.
thread = notmuch_threads_get (threads);
notmuch_thread_destroy (thread);
*/
i++;
if (i > 100) break;
}
for (;
notmuch_threads_valid (threads);
notmuch_threads_move_to_next (threads))
{
/* the thread acquired here will be the first thread in the query.
* it should be the 101th. */
thread = notmuch_threads_get (threads);
notmuch_thread_destroy (thread);
}
notmuch_query_destroy (query);
```
It is quite slow to skip the threads in this way, might it be faster if
move_to_next works correctly?
Regards, Gaute
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v3 13/16] add indexopts to notmuch python bindings.
Daniel Kahn Gillmor writes: > Make notmuch indexing options are not available in python as > the notmuch.Indexopts class. Users can do something like: I'd like to have feedback from Justus before I merge this. That need not block the rest of the series I guess? d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: need to call notmuch_threads_get (..) to actually move iterator
Gaute Hope writes on February 28, 2016 13:36: Gaute Hope writes on February 24, 2016 13:08: Hi, it seems to be necessary to actually call notmuch_threads_get (threads) to move the thread iterator from a query object, just calling notmuch_threads_move_to_next (..) is not enough: A test-case demonstrating this (for the 'astroid' code-base) is located here: https://github.com/gauteh/astroid/blob/ti-stateless-query/test/test_notmuch.cc#L66 At the moment, this test depends on the rest of the astroid test-suite, but it may be useful as a demonstration or skeleton for a notmuch test-case anyway. Disregard that, my test-case was erroneous. -Gaute ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: need to call notmuch_threads_get (..) to actually move iterator
Gaute Hope writes on February 24, 2016 13:08: Hi, it seems to be necessary to actually call notmuch_threads_get (threads) to move the thread iterator from a query object, just calling notmuch_threads_move_to_next (..) is not enough: A test-case demonstrating this (for the 'astroid' code-base) is located here: https://github.com/gauteh/astroid/blob/ti-stateless-query/test/test_notmuch.cc#L66 At the moment, this test depends on the rest of the astroid test-suite, but it may be useful as a demonstration or skeleton for a notmuch test-case anyway. Regards, Gaute ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: how do the different frontends deal with displaying large queries?
David Bremner writes on February 27, 2016 13:50: Gaute Hope writes: While loading the threads, if I make enough modifications to the database while the query is still loading (specifically 1: removing the unread tag from a thread, and 2: adding the unread tag to the same thread) apparently the query is invalidated in some way, and I get a hard crash in `notmuch_tags_get ()`. The thread in question has alreaday been loaded and is displayed. Do you deal with this issue in some way? I think we "deal with" it by not leaving the database open very long. The query is dumped as s-expr by a seperate process. Or perhaps we just aren't doing the same level of concurrent operations, I don't know. Neither do I, I load the threads in the background. But for a long query that takes a while. The database is only kept open for the duration it takes to load the threads in the query. How long is the db open? Would you notice if it crashed? I'm guessing the abort is triggered by an Xapian::ModifiedDatabase somewhere. I am only able to consistently reproduce the error if I change the tags of a thread not yet loaded, a (somewhat messy) test case is available here: https://github.com/gauteh/astroid/blob/ti-stateless-query/test/test_notmuch_standalone.cc If nothing else, it demonstrates the difficulty of handling these exceptions in functions that don't yet catch them, since they could happen along many of the internal steps. Regards, Gaute ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
