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

[Tatsuo Ishii]

> 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 --------------
A non-text attachment was scrubbed...
Name: memqcache.patch
Type: text/x-patch
Size: 46841 bytes
Desc: not available
URL: <http://pgfoundry.org/pipermail/pgpool-hackers/attachments/20110628/236bcac3/attachment-0001.bin>