Re: [Dbmail-dev] search to optimize _ic_search

Fri, 11 Jun 2004 21:33:13 +0200 (CEST) 


Aaron Stone wrote:

_ic_fetch really need to be broken down into its parts. Roel and I talked
about how it got so ugly, basically he wrote the IMAP daemon incrementally and
never got to a refactoring cycle once he reached the functionality goals.


The good-old bottom-up approach.


This would be a very good project for someone new out there who's looking to
get their hands dirty with core DBMail code! Even if someone currently more
involved and familiar with the code does the actual final version, this would
still be a good place for someone new to get their feet wet (and legs, and
pretty much everything by jumping into cold water).



Sounds like a recipe for failure. Not exactly motivating to spend your 
time sorting through a bunch of spagetti to see your patches go 
basically unused.



To really stress test it, I sorted my INBOX by the Sender header. It was
actually really quick, just five minutes and 10,000 or 15,000 queries to sort
through my 5,000 message INBOX ;-)  But hey, it worked!



One query for the actual sort should do. IMO, anything else is 
suboptimal and a sign of poor design.



Header caching will probably necessitate a rethinking of _ic_fetch more than
just a refactoring... but refactoring first will make it much easier to avoid
the side effects of rewriting complex code for new features and clarity.



It's a dirty job. But someone's gotta do it, I guess. I will expand my 
is_search patch.





Aaron


Paul J Stevens <[EMAIL PROTECTED]> said:



I just found out why retrieval suddenly broke.


Apparently when I change db_fetch_headers to actually retrieve nothing 
but headers, _ic_fetch breaks because it expects db_fetch_header to 
actually retrieve and parse the whole message. Messed-up semantics I guess.


Also, globals like msgbuf and cached_msg are busting my balls.


If I understand the code correctly, dbmail currently can handle only a 
single parsed messaged at any one moment.



If we can handle lists of parsed messages I see room for massive 
performance enhancements since we can then fase in queries like


' select ... from ... where message_idnr in (uid1,uid2,...uidn) '


which the backend handles very well, instead of retrieving messages one 
at a time. Many joins where query-sequences are used are also possible. 
Many orders of magnitude faster we can and must make dbmail by using 
smarter SQL.



Paul J Stevens wrote:


Oops. Message retrieval is broken in this patch. Don't use it just yet.

boyohboy, _ic_fetch sure is butt-ugly :-)



Paul J Stevens wrote:




Aaron Stone wrote:



db_insert_message_block() and db_insert_message_block_physmessage().





Ilja,

Hang in there. Full patch for is_header flag attached.





------------------------------------------------------------------------

#! /bin/sh -e
## 07_is_header.dpatch by  <[EMAIL PROTECTED]>
##
## All lines beginning with `## DP:' are a description of the patch.
## DP: No description.

if [ $# -lt 1 ]; then
   echo "`basename $0`: script expects -patch|-unpatch as argument" >&2
   exit 1
fi

[ -f debian/patches/00patch-opts ] && . debian/patches/00patch-opts
patch_opts="${patch_opts:--f --no-backup-if-mismatch} ${2:+-d $2}"

case "$1" in
   -patch) patch -p1 ${patch_opts} < $0;;
   -unpatch) patch -R -p1 ${patch_opts} < $0;;
   *)

       echo "`basename $0`: script expects -patch|-unpatch as 
argument" >&2

       exit 1;;
esac

exit 0

@DPATCH@
diff -urNad /usr/src/dbmail2/dbmail-2.0/db.c dbmail-2.0/db.c

--- /usr/src/dbmail2/dbmail-2.0/db.c    2004-06-10 14:32:51.000000000 
+0200

+++ dbmail-2.0/db.c    2004-06-10 14:32:51.000000000 +0200
@@ -911,7 +911,8 @@
int db_insert_message_block_physmessage(const char *block,
                    u64_t block_size,
                    u64_t physmessage_id,
-                    u64_t * messageblk_idnr)
+                    u64_t * messageblk_idnr,
+                    unsigned is_header)
{
    char *escaped_query = NULL;
    unsigned maxesclen = (READ_BLOCK_SIZE + 1) * 2 + DEF_QUERYSIZE;
@@ -947,7 +948,8 @@
    startlen =
        snprintf(escaped_query, maxesclen,
             "INSERT INTO messageblks"
-             "(messageblk,blocksize, physmessage_id) VALUES ('");

+             "(is_header,messageblk,blocksize, physmessage_id) VALUES 
('%u','", +             is_header);
   
    /* escape & add data */

    esclen =
@@ -973,7 +975,7 @@
}

int db_insert_message_block(const char *block, u64_t block_size,
-                u64_t message_idnr, u64_t * messageblk_idnr)

+                u64_t message_idnr, u64_t * messageblk_idnr, unsigned 
is_header)

{
    u64_t physmessage_id;

@@ -993,7 +995,7 @@
    }

    if (db_insert_message_block_physmessage
-        (block, block_size, physmessage_id, messageblk_idnr) < 0) {

+        (block, block_size, physmessage_id, messageblk_idnr, 
is_header) < 0) {

        trace(TRACE_ERROR,
              "%s,%s: error inserting messageblks for "
              "physmessage [%llu]", __FILE__, __func__,
@@ -2052,9 +2054,9 @@

        if (db_insert_message_block_physmessage(msgdata, datalen,
                            physmessage_id,
-                            &messageblk_idnr) == -1
+                            &messageblk_idnr,1) == -1
            || db_insert_message_block(" \n", 2, message_idnr,
-                           &messageblk_idnr) == -1) {
+                           &messageblk_idnr,1) == -1) {
            trace(TRACE_ERROR,
                  "%s,%s: could not insert msg block\n",
                  __FILE__, __func__);
@@ -2077,7 +2079,7 @@
        if (db_insert_message_block_physmessage(
                msgdata, count,                  physmessage_id,
-                &messageblk_idnr) == -1) {
+                &messageblk_idnr,1) == -1) {
            trace(TRACE_ERROR,
                  "%s,%s: could not insert msg block\n",
                  __FILE__, __func__);
@@ -2097,7 +2099,7 @@
                    &msgdata[count],
                    READ_BLOCK_SIZE,
                    physmessage_id,
-                    &messageblk_idnr) == -1) {
+                    &messageblk_idnr,0) == -1) {
                trace(TRACE_ERROR,
                      "%s,%s: could not insert msg block",
                      __FILE__, __func__);
@@ -2117,7 +2119,7 @@
        if (db_insert_message_block_physmessage(
                &msgdata[count],
                datalen - count, physmessage_id,
-                &messageblk_idnr) == -1) {
+                &messageblk_idnr,0) == -1) {
            trace(TRACE_ERROR,
                  "%s,%s:  could not insert msg block\n",
                  __FILE__, __func__);
diff -urNad /usr/src/dbmail2/dbmail-2.0/db.h dbmail-2.0/db.h

--- /usr/src/dbmail2/dbmail-2.0/db.h    2004-06-10 14:30:50.000000000 
+0200

+++ dbmail-2.0/db.h    2004-06-10 14:32:51.000000000 +0200
@@ -613,7 +613,8 @@
int db_insert_message_block_physmessage(const char *block,
                    u64_t block_size,
                    u64_t physmessage_id,
-                    u64_t * messageblock_idnr);
+                    u64_t * messageblock_idnr,
+                    unsigned is_header);
/**
* \brief insert a message block into the message block table
* \param block the message block (which is a string)
@@ -627,7 +628,8 @@
*/
int db_insert_message_block(const char *block, u64_t block_size,

                u64_t message_idnr, -                /[EMAIL PROTECTED]@*/ u64_t 
* messageblock_idnr);

+                /[EMAIL PROTECTED]@*/ u64_t * messageblock_idnr,
+                unsigned is_header);
/**
 * \brief log IP-address for POP/IMAP_BEFORE_SMTP. If the IP-address
 *        is already logged, it's timestamp is renewed.
diff -urNad /usr/src/dbmail2/dbmail-2.0/dbmsgbuf.c dbmail-2.0/dbmsgbuf.c

--- /usr/src/dbmail2/dbmail-2.0/dbmsgbuf.c    2004-06-10 
14:30:50.000000000 +0200

+++ dbmail-2.0/dbmsgbuf.c    2004-06-10 14:32:51.000000000 +0200
@@ -53,7 +53,8 @@
static unsigned nblocks = 0; /**< number of block  */
static const char * tmprow; /**< temporary row number */


-int db_init_msgfetch(u64_t msg_idnr) +int 
db_init_fetch_messageblks(u64_t msg_idnr, char *query_template);

+int db_init_fetch_messageblks(u64_t msg_idnr, char *query_template)
{

    msgbuf_buf = (char *) my_malloc(sizeof(char) * (size_t) 
MSGBUF_WINDOWSIZE);

    if (!msgbuf_buf) {
@@ -64,12 +65,7 @@
        return 0;
    memset(msgbuf_buf, '\0', (size_t) MSGBUF_WINDOWSIZE);

   
-    snprintf(query, DEF_QUERYSIZE,

-        "SELECT messageblk FROM messageblks LEFT JOIN messages "
-        "USING (physmessage_id) "
-        "WHERE messages.message_idnr = '%llu' "
-        "ORDER BY messageblk_idnr",
-        msg_idnr);
+    snprintf(query, DEF_QUERYSIZE, query_template, msg_idnr);

    if (db_query(query) == -1) {
        trace(TRACE_ERROR, "%s,%s: could not get message",
@@ -143,6 +139,23 @@
     * later use */
    db_store_msgbuf_result();
    return 1;
+
+}
+
+int db_init_fetch_headers(u64_t msg_idnr)
+{

+    char *query_template = "SELECT messageblk FROM messageblks LEFT 
JOIN messages "

+        "USING (physmessage_id) WHERE messages.message_idnr = '%llu' "
+        "AND messageblks.is_header = 1";
+    return db_init_fetch_messageblks(msg_idnr, query_template);
+
+}
+int db_init_fetch_message(u64_t msg_idnr) +{

+    char *query_template = "SELECT messageblk FROM messageblks LEFT 
JOIN messages "

+        "USING (physmessage_id) WHERE messages.message_idnr = '%llu' "
+        "ORDER BY messageblk_idnr";
+    return db_init_fetch_messageblks(msg_idnr, query_template);
}


   
diff -urNad /usr/src/dbmail2/dbmail-2.0/dbmsgbuf.h dbmail-2.0/dbmsgbuf.h
--- /usr/src/dbmail2/dbmail-2.0/dbmsgbuf.h    2004-06-10 
14:30:50.000000000 +0200

+++ dbmail-2.0/dbmsgbuf.h    2004-06-10 14:32:51.000000000 +0200
@@ -50,7 +50,18 @@
 *     -  0 if already inited (sic) before
 *     -  1 on success
 */
-int db_init_msgfetch(u64_t msg_idnr);
+int db_init_fetch_message(u64_t msg_idnr);
+
+/**
+ * \brief initialises a message headers fetch
+ * \param msg_idnr + * \return + *     - -1 on error
+ *     -  0 if already inited (sic) before
+ *     -  1 on success
+ */
+int db_init_fetch_headers(u64_t msg_idnr);
+

/**
 * \brief update msgbuf
diff -urNad /usr/src/dbmail2/dbmail-2.0/pipe.c dbmail-2.0/pipe.c

--- /usr/src/dbmail2/dbmail-2.0/pipe.c    2004-06-10 
14:30:50.000000000 +0200

+++ dbmail-2.0/pipe.c    2004-06-10 14:32:51.000000000 +0200
@@ -70,7 +70,7 @@
 *     - -1 on error
 *     -  1 on success
 */
-static int store_message_in_blocks(const char* message,
+static int db_store_body_blocks(const char* message,
                   u64_t message_size,
                   u64_t msgidnr);

@@ -413,7 +413,7 @@
    }

    switch (db_insert_message_block
-        (header, headersize, msgidnr, &messageblk_idnr)) {
+        (header, headersize, msgidnr, &messageblk_idnr,1)) {
    case -1:
        trace(TRACE_ERROR,

              "store_message_temp(): error inserting msgblock 
[header]");

@@ -424,7 +424,7 @@
          "for readblock", READ_BLOCK_SIZE);

   
    /* store body in several blocks (if needed */

-    if (store_message_in_blocks(body, bodysize, msgidnr) < 0) {
+    if (db_store_body_blocks(body, bodysize, msgidnr) < 0) {
        trace(TRACE_STOP,
              "store_message_temp(): db_insert_message_block "
              "failed");
@@ -447,7 +447,7 @@
    return 1;
}

-int store_message_in_blocks(const char *message, u64_t message_size,
+int db_store_body_blocks(const char *message, u64_t message_size,
                u64_t msgidnr)  {
    u64_t tmp_messageblk_idnr;
@@ -466,7 +466,7 @@
              __FILE__, __func__, &message[offset]);
        if (db_insert_message_block(&message[offset],
                        block_size, msgidnr,
-                        &tmp_messageblk_idnr) < 0) {
+                        &tmp_messageblk_idnr,0) < 0) {
            trace(TRACE_ERROR, "%s,%s: "
                  "db_insert_message_block() failed",
                  __FILE__, __func__);
diff -urNad /usr/src/dbmail2/dbmail-2.0/rfcmsg.c dbmail-2.0/rfcmsg.c

--- /usr/src/dbmail2/dbmail-2.0/rfcmsg.c    2004-06-10 
14:30:50.000000000 +0200

+++ dbmail-2.0/rfcmsg.c    2004-06-10 14:32:51.000000000 +0200
@@ -117,7 +117,7 @@
{
    int result, level = 0, maxlevel = -1;

-    if (db_init_msgfetch(msguid) != 1) {
+    if (db_init_fetch_headers(msguid) != 1) {
        trace(TRACE_ERROR,
              "db_fetch_headers(): could not init msgfetch\n");
        return -2;
@@ -148,7 +148,7 @@

                  "db_fetch_headers(): trying to fetch at maxlevel 
%d...\n",

                  maxlevel);

-            if (db_init_msgfetch(msguid) != 1) {
+            if (db_init_fetch_message(msguid) != 1) {
                trace(TRACE_ERROR,
                      "db_fetch_headers(): could not init msgfetch\n");
                return -2;
@@ -180,7 +180,7 @@


        /* ok still problems... try to make a message */
-        if (db_init_msgfetch(msguid) != 1) {
+        if (db_init_fetch_message(msguid) != 1) {
            trace(TRACE_ERROR,
                  "db_fetch_headers(): could not init msgfetch\n");
            return -2;

diff -urNad /usr/src/dbmail2/dbmail-2.0/smtp-convert.c 
dbmail-2.0/smtp-convert.c
--- /usr/src/dbmail2/dbmail-2.0/smtp-convert.c    2004-06-10 
14:30:50.000000000 +0200

+++ dbmail-2.0/smtp-convert.c    2004-06-10 14:32:51.000000000 +0200
@@ -195,7 +195,7 @@
                in_msg = 1;    /* ok start of a new msg */
            else {
                /* update & end message */
-                db_insert_message_block(blk, cnt, msgid);
+                db_insert_message_block(blk, cnt, msgid,0);

                create_unique_id(newunique, msgid);
                db_update_message(msgid, newunique,
@@ -222,7 +222,7 @@
                if (strcmp(&blk[cnt], "\n") == 0) {
                    db_insert_message_block(blk,
                                cnt + len,
-                                msgid);
+                                msgid,1);
                    header_passed = 1;
                    size += (cnt + len);
                    cnt = 0;
@@ -240,7 +240,7 @@
                    blk[READ_BLOCK_SIZE] = '\0';
                    db_insert_message_block(blk,
                                READ_BLOCK_SIZE,
-                                msgid);
+                                msgid,0);
                    blk[READ_BLOCK_SIZE] = saved;

                    memmove(blk, &blk[READ_BLOCK_SIZE],
@@ -254,7 +254,7 @@

    /* update & end message */
    if (msgid > 0) {
-        db_insert_message_block(blk, cnt, msgid);
+        db_insert_message_block(blk, cnt, msgid,0);

        create_unique_id(newunique, msgid);
        db_update_message(msgid, newunique, size + cnt,


------------------------------------------------------------------------

_______________________________________________
Dbmail-dev mailing list
Dbmail-dev@dbmail.org
http://twister.fastxs.net/mailman/listinfo/dbmail-dev




--
  ________________________________________________________________
  Paul Stevens                                  mailto:[EMAIL PROTECTED]
  NET FACILITIES GROUP                     PGP: finger [EMAIL PROTECTED]
  The Netherlands________________________________http://www.nfg.nl
_______________________________________________
Dbmail-dev mailing list
Dbmail-dev@dbmail.org
http://twister.fastxs.net/mailman/listinfo/dbmail-dev








--
  ________________________________________________________________
  Paul Stevens                                  mailto:[EMAIL PROTECTED]
  NET FACILITIES GROUP                     PGP: finger [EMAIL PROTECTED]
  The Netherlands________________________________http://www.nfg.nl


























					Reply via email to