[Pgpool-hackers] patch "Caching query results in pgpool-II"

[Masanori Yamazaki]

Thank you very much for lots of useful advice.

> If the meaning of the flag "is_select_non_immutable_flag" is "true if
> the SELECT statement includes non IMMUTABLE function", the apparently
> the test is wrong.

I mistook how to use non IMMUTABLE function, first I corrected code
regarding it.

> Did you turn on -d while testing your code?

Yes, I do. But I mainly saw and check the values of variables running GDB.
for example.

(gdb) b ProcessBackendResponse
Breakpoint 1 at 0x8089895: file pool_proto_modules.c, line 2092.
(gdb) n
Single stepping until exit from function __kernel_vsyscall,
which has no line number information.
0x00272fbd in ___newselect_nocancel () from /lib/libc.so.6
(gdb) n
...
(gdb) s

Breakpoint 1, ProcessBackendResponse (frontend=0x8648640, backend=0x8647e28,

    state=0xbfe4af74, num_fields=0xbfe4af7a) at pool_proto_modules.c:2092
2092    {
(gdb) n
2098            session_context = pool_get_session_context();
(gdb) p *backend->info
$2 = {database = "sample", '\000' <repeats 57 times>,
  user = "postgres", '\000' <repeats 23 times>, major = 3, minor = 0,
  pid = 1948712960, key = -604132037, counter = 1, create_time = 1307271851,

  load_balancing_node = 0, connected = 1 '\001'}
(gdb)
...

I will take advantage of the debug log file or pool_debug from now on.
I refer to other proper comments and correct some comments like "@param",
and add more comments to code because it is true that there are few comments
in code.





2011/6/28 Tatsuo Ishii <ishii at sraoss.co.jp>

> > Other points after reviewing your code.
> >
> > First of all I want to make sure that what are not included in the
> > patches:
> >
> > 1) Shared memory cache
> > 2) Cache data when extended protocol is used
> > 3) Cache invalidation when receives DDL/DML related to query cache
> > 4) Cache should not be registered if transaction aborts
> > 5) Cache should not be registered if the table is temporary table
> > 6) configure does not recognize "--with-memcached" option. This part,
> >    I will write a patch.
>
> I have written patches for this. Attached is the patches *including*
> Masanori's patches against CVS HEAD (memqcache.[ch] are not included).
>
> Usage:
>
> ./configure --with-memcached=/usr/local
>
> Masanori, you need to do this:
>
> > 7) All memory cache codes using memcached should be surrounded by
> >    #ifdef USE_MEMCACHED ... #endif
> >
> > 8) directive "memqcache_dir" is missing
> >
> > Second, questionable codes in the patches:
> >
> > 1) No memory allocation error check in add_buf.
> >
> > 2) Excess memory allocation in add_buf. If it needs n byte, it allways
> >    allocate memqcache_total_size + n bytes. Why do we need extra
> >    memqcache_total_size?
> >
> > 3) What is this code doing? "con" is never
> >    used. memqcache_total_size's 3rd arg is "oid", while you are
> >    passing a file descriptor.
> >
> >       /* save the received result to buf for each kind */
> >       if (pool_config->memory_cache_enabled)
> >       {
> >               char* con;
> >               memqcache_register(kind, frontend, frontend->fd, p1, len1);
> >       }
> >
> > 4) write_tableoid()
> >
> >    - directory should be obtained from memqcache_dir directive
> >
> >    - file extention ."txt" is not appropreate becauase the file does
> >      not need to be a text file.
> >
> >    - writing md5 hash key as a string is wate of space. Just use
> fwrite().
> >
> >    - The function should store md5 hash key + cache ids. Without cache
> >      ids, how can you invalidate cache entries?
> >
> > 5) About cache key
> >
> >    Cache key should be md5(username+query_string+database_name), not
> >    just query_string. Also we need to store username at the same time
> >    to make sure that the cache data is not accessed by someone
> >    else. Otherwise we will have a security concern. See discussions in
> >    pgpool-hackers.
> >
> >
> http://lists.pgfoundry.org/pipermail/pgpool-hackers/2011-June/000817.html
> > --
> > Tatsuo Ishii
> > SRA OSS, Inc. Japan
> > English: http://www.sraoss.co.jp/index_en.php
> > Japanese: http://www.sraoss.co.jp
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://pgfoundry.org/pipermail/pgpool-hackers/attachments/20110629/d992a92f/attachment.html>