Commit f62cc1e6 authored by unknown's avatar unknown

Discoved while debugging in 5.1 that there was a bug where a certain crash...

Discoved while debugging in 5.1 that there was a bug where a certain crash could lead to two problems. 1) An additional share in memory that was allocated but did not have the correct use_count (so it would never be fulled deleted). Also discovered that a thread that called repair would write new rows, but would not see them. All other threads were ok, and the data was fine, but the thread doing the repair was unable to see the new rows.


sql/ha_archive.cc:
  Fix for leaked share and hidden rows.
sql/ha_archive.h:
  Change in method
parent 4e8f2a12
...@@ -366,12 +366,14 @@ int ha_archive::write_meta_file(File meta_file, ha_rows rows, bool dirty) ...@@ -366,12 +366,14 @@ int ha_archive::write_meta_file(File meta_file, ha_rows rows, bool dirty)
See ha_example.cc for a longer description. See ha_example.cc for a longer description.
*/ */
ARCHIVE_SHARE *ha_archive::get_share(const char *table_name, TABLE *table) ARCHIVE_SHARE *ha_archive::get_share(const char *table_name,
TABLE *table, int *rc)
{ {
ARCHIVE_SHARE *share; ARCHIVE_SHARE *share;
char meta_file_name[FN_REFLEN]; char meta_file_name[FN_REFLEN];
uint length; uint length;
char *tmp_name; char *tmp_name;
DBUG_ENTER("ha_archive::get_share");
pthread_mutex_lock(&archive_mutex); pthread_mutex_lock(&archive_mutex);
length=(uint) strlen(table_name); length=(uint) strlen(table_name);
...@@ -386,7 +388,8 @@ ARCHIVE_SHARE *ha_archive::get_share(const char *table_name, TABLE *table) ...@@ -386,7 +388,8 @@ ARCHIVE_SHARE *ha_archive::get_share(const char *table_name, TABLE *table)
NullS)) NullS))
{ {
pthread_mutex_unlock(&archive_mutex); pthread_mutex_unlock(&archive_mutex);
return NULL; *rc= HA_ERR_OUT_OF_MEM;
DBUG_RETURN(NULL);
} }
share->use_count= 0; share->use_count= 0;
...@@ -424,9 +427,14 @@ ARCHIVE_SHARE *ha_archive::get_share(const char *table_name, TABLE *table) ...@@ -424,9 +427,14 @@ ARCHIVE_SHARE *ha_archive::get_share(const char *table_name, TABLE *table)
thr_lock_init(&share->lock); thr_lock_init(&share->lock);
} }
share->use_count++; share->use_count++;
DBUG_PRINT("info", ("archive table %.*s has %d open handles now",
share->table_name_length, share->table_name,
share->use_count));
if (share->crashed)
*rc= HA_ERR_CRASHED_ON_USAGE;
pthread_mutex_unlock(&archive_mutex); pthread_mutex_unlock(&archive_mutex);
return share; DBUG_RETURN(share);
} }
...@@ -437,13 +445,21 @@ ARCHIVE_SHARE *ha_archive::get_share(const char *table_name, TABLE *table) ...@@ -437,13 +445,21 @@ ARCHIVE_SHARE *ha_archive::get_share(const char *table_name, TABLE *table)
int ha_archive::free_share(ARCHIVE_SHARE *share) int ha_archive::free_share(ARCHIVE_SHARE *share)
{ {
int rc= 0; int rc= 0;
DBUG_ENTER("ha_archive::free_share");
DBUG_PRINT("info", ("archive table %.*s has %d open handles on entrance",
share->table_name_length, share->table_name,
share->use_count));
pthread_mutex_lock(&archive_mutex); pthread_mutex_lock(&archive_mutex);
if (!--share->use_count) if (!--share->use_count)
{ {
hash_delete(&archive_open_tables, (byte*) share); hash_delete(&archive_open_tables, (byte*) share);
thr_lock_delete(&share->lock); thr_lock_delete(&share->lock);
VOID(pthread_mutex_destroy(&share->mutex)); VOID(pthread_mutex_destroy(&share->mutex));
(void)write_meta_file(share->meta_file, share->rows_recorded, FALSE); if (share->crashed)
(void)write_meta_file(share->meta_file, share->rows_recorded, TRUE);
else
(void)write_meta_file(share->meta_file, share->rows_recorded, FALSE);
if (gzclose(share->archive_write) == Z_ERRNO) if (gzclose(share->archive_write) == Z_ERRNO)
rc= 1; rc= 1;
if (my_close(share->meta_file, MYF(0))) if (my_close(share->meta_file, MYF(0)))
...@@ -452,7 +468,7 @@ int ha_archive::free_share(ARCHIVE_SHARE *share) ...@@ -452,7 +468,7 @@ int ha_archive::free_share(ARCHIVE_SHARE *share)
} }
pthread_mutex_unlock(&archive_mutex); pthread_mutex_unlock(&archive_mutex);
return rc; DBUG_RETURN(rc);
} }
...@@ -479,10 +495,23 @@ const char **ha_archive::bas_ext() const ...@@ -479,10 +495,23 @@ const char **ha_archive::bas_ext() const
*/ */
int ha_archive::open(const char *name, int mode, uint open_options) int ha_archive::open(const char *name, int mode, uint open_options)
{ {
int rc= 0;
DBUG_ENTER("ha_archive::open"); DBUG_ENTER("ha_archive::open");
if (!(share= get_share(name, table))) DBUG_PRINT("info", ("archive table was opened for crash %s",
DBUG_RETURN(HA_ERR_OUT_OF_MEM); // Not handled well by calling code! (open_options & HA_OPEN_FOR_REPAIR) ? "yes" : "no"));
share= get_share(name, table, &rc);
if (rc == HA_ERR_CRASHED_ON_USAGE && !(open_options & HA_OPEN_FOR_REPAIR))
{
free_share(share);
DBUG_RETURN(rc);
}
else if (rc == HA_ERR_OUT_OF_MEM)
{
DBUG_RETURN(rc);
}
thr_lock_data_init(&share->lock,&lock,NULL); thr_lock_data_init(&share->lock,&lock,NULL);
if ((archive= gzopen(share->data_file_name, "rb")) == NULL) if ((archive= gzopen(share->data_file_name, "rb")) == NULL)
...@@ -492,10 +521,14 @@ int ha_archive::open(const char *name, int mode, uint open_options) ...@@ -492,10 +521,14 @@ int ha_archive::open(const char *name, int mode, uint open_options)
DBUG_RETURN(HA_ERR_CRASHED_ON_USAGE); DBUG_RETURN(HA_ERR_CRASHED_ON_USAGE);
} }
if (open_options & HA_OPEN_FOR_REPAIR) DBUG_PRINT("info", ("archive table was crashed %s",
rc == HA_ERR_CRASHED_ON_USAGE ? "yes" : "no"));
if (rc == HA_ERR_CRASHED_ON_USAGE && open_options & HA_OPEN_FOR_REPAIR)
{
DBUG_RETURN(0); DBUG_RETURN(0);
}
DBUG_RETURN(share->crashed ? HA_ERR_CRASHED_ON_USAGE : 0); else
DBUG_RETURN(rc);
} }
...@@ -684,6 +717,7 @@ int ha_archive::rnd_init(bool scan) ...@@ -684,6 +717,7 @@ int ha_archive::rnd_init(bool scan)
if (scan) if (scan)
{ {
scan_rows= share->rows_recorded; scan_rows= share->rows_recorded;
DBUG_PRINT("info", ("archive will retrieve %llu rows", scan_rows));
records= 0; records= 0;
/* /*
...@@ -695,6 +729,7 @@ int ha_archive::rnd_init(bool scan) ...@@ -695,6 +729,7 @@ int ha_archive::rnd_init(bool scan)
pthread_mutex_lock(&share->mutex); pthread_mutex_lock(&share->mutex);
if (share->dirty == TRUE) if (share->dirty == TRUE)
{ {
DBUG_PRINT("info", ("archive flushing out rows for scan"));
gzflush(share->archive_write, Z_SYNC_FLUSH); gzflush(share->archive_write, Z_SYNC_FLUSH);
share->dirty= FALSE; share->dirty= FALSE;
} }
...@@ -913,6 +948,7 @@ int ha_archive::optimize(THD* thd, HA_CHECK_OPT* check_opt) ...@@ -913,6 +948,7 @@ int ha_archive::optimize(THD* thd, HA_CHECK_OPT* check_opt)
share->rows_recorded++; share->rows_recorded++;
} }
} }
DBUG_PRINT("info", ("recovered %llu archive rows", share->rows_recorded));
my_free((char*)buf, MYF(0)); my_free((char*)buf, MYF(0));
if (rc && rc != HA_ERR_END_OF_FILE) if (rc && rc != HA_ERR_END_OF_FILE)
...@@ -936,11 +972,23 @@ int ha_archive::optimize(THD* thd, HA_CHECK_OPT* check_opt) ...@@ -936,11 +972,23 @@ int ha_archive::optimize(THD* thd, HA_CHECK_OPT* check_opt)
} }
gzflush(writer, Z_SYNC_FLUSH); gzflush(writer, Z_SYNC_FLUSH);
share->dirty= FALSE;
gzclose(share->archive_write); gzclose(share->archive_write);
share->archive_write= writer; share->archive_write= writer;
my_rename(writer_filename,share->data_file_name,MYF(0)); my_rename(writer_filename,share->data_file_name,MYF(0));
/*
Now we need to reopen our read descriptor since it has changed.
*/
gzclose(archive);
if ((archive= gzopen(share->data_file_name, "rb")) == NULL)
{
rc= HA_ERR_CRASHED_ON_USAGE;
goto error;
}
DBUG_RETURN(0); DBUG_RETURN(0);
error: error:
......
...@@ -85,7 +85,7 @@ class ha_archive: public handler ...@@ -85,7 +85,7 @@ class ha_archive: public handler
int get_row(gzFile file_to_read, byte *buf); int get_row(gzFile file_to_read, byte *buf);
int read_meta_file(File meta_file, ha_rows *rows); int read_meta_file(File meta_file, ha_rows *rows);
int write_meta_file(File meta_file, ha_rows rows, bool dirty); int write_meta_file(File meta_file, ha_rows rows, bool dirty);
ARCHIVE_SHARE *get_share(const char *table_name, TABLE *table); ARCHIVE_SHARE *get_share(const char *table_name, TABLE *table, int *rc);
int free_share(ARCHIVE_SHARE *share); int free_share(ARCHIVE_SHARE *share);
bool auto_repair() const { return 1; } // For the moment we just do this bool auto_repair() const { return 1; } // For the moment we just do this
int read_data_header(gzFile file_to_read); int read_data_header(gzFile file_to_read);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment