#At file:///ext/mysql/bzr/backup/bug45093/ based on revid:jorgen.loland@stripped
2833 Rafal Somla 2009-06-26
Bug #45093 - Wrong logic in backup stream library in bstream_next_chunk()...
The problem in bstream_next_chunk() was that the logic did not take into
account the fact that there can be more than one fragment of a chunk left
in the stream - only the current fragment was skipped.
This patch adds a loop which skips all remaining fragments of the current
chunk in order to move to the next one. It also fixes few other small
problems detected when I worked on this issue.
@ sql/backup/stream_v1.c
Small fixes:
- in bstream_wr_db_catalogue() function, db_iterator is created and thus
db_iterator_free() function should be called to free it.
- no need to skip tables in the loop iterating over per-table items:
prbably a copy-paste error.
@ sql/backup/stream_v1_transport.c
The main fix is in bstream_next_chunk() function where a loop is added. This
loop will skip all remaining fragments of the current chunk, so that stream
is positioned at the beginning of next chunk.
Apart from that:
- Fix definition of FR_LEN_MASK to specify bits explicitly. Otherwise the
bit pattern could vary depending on the platform.
- Fix logic detecting end-of-chunk in load_next_fragment() function.
- Make check_end() function correctly set stream state when end-of-chunk
is detected.
modified:
sql/backup/stream_v1.c
sql/backup/stream_v1.h
sql/backup/stream_v1_transport.c
=== modified file 'sql/backup/stream_v1.c'
--- a/sql/backup/stream_v1.c 2009-05-25 07:11:29 +0000
+++ b/sql/backup/stream_v1.c 2009-06-26 06:28:20 +0000
@@ -1062,7 +1062,7 @@ int bstream_wr_db_catalogue(backup_strea
/* Free iterator if not already done */
if (iter)
- bcat_iterator_free(cat,iter);
+ bcat_db_iterator_free(cat, db_info, iter);
return ret;
}
@@ -1323,12 +1323,7 @@ int bstream_wr_meta_data(backup_stream *
return BSTREAM_ERROR;
while ((item= bcat_iterator_next(cat,iter)))
- {
- if (item->type == BSTREAM_IT_TABLE)
- continue;
-
CHECK_WR_RES(bstream_wr_item_def(s,cat,PER_TABLE_ITEM,item));
- }
wr_error:
=== modified file 'sql/backup/stream_v1.h'
--- a/sql/backup/stream_v1.h 2009-05-25 07:11:29 +0000
+++ b/sql/backup/stream_v1.h 2009-06-26 06:28:20 +0000
@@ -421,10 +421,10 @@ struct st_abstract_stream
*/
struct st_bstream_buffer
{
- bstream_byte *begin; /**< pointer to start of stream buffer */
- bstream_byte *pos; /**< current position in buffer */
- bstream_byte *header; /**< pointer to header */
- bstream_byte *end; /**< pointer to end of stream buffer */
+ bstream_byte *begin; /**< pointer to start of stream buffer */
+ bstream_byte *pos; /**< current position in buffer */
+ bstream_byte *header; /**< pointer to the header of the next fragment */
+ bstream_byte *end; /**< pointer to the end of the current stream block */
};
/**
=== modified file 'sql/backup/stream_v1_transport.c'
--- a/sql/backup/stream_v1_transport.c 2009-05-25 07:11:29 +0000
+++ b/sql/backup/stream_v1_transport.c 2009-06-26 06:28:20 +0000
@@ -180,7 +180,7 @@ extern byte get_byte_size_t(size_t value
#define FR_LAST 0x40 /**< bits for LAST fragment */
#define FR_TYPE_MASK 0xC0 /**< type bits for mask */
-#define FR_LEN_MASK (~FR_TYPE_MASK) /**< type length for mask */
+#define FR_LEN_MASK 0x3F /**< type length for mask */
/** biggest size of small fragment */
#define FR_SMALL_MAX ((size_t)FR_LEN_MASK)
@@ -351,21 +351,21 @@ int as_read_all(struct st_abstract_strea
It stores data to be written to the next block in the output stream. Data
in block is divided into fragments. If buffer is non-empty, the header of
- last fragment stored in it is pointed by header. pos marks the end of data
- in the buffer, end indicates how much bytes is left until the end of output
- block
+ last fragment stored in it is pointed by buf.header. Buf.pos marks the end of
+ data in the buffer, buf.end indicates how much bytes is left until the end of
+ output block
other fragments current fragment free space
[===================|XX:=============:..............]
^ ^ ^ ^
- begin header pos end
+ buf.begin buf.header buf.pos buf.end
- Note: pos == header means that the current fragment is empty and when appending
- data to it, one byte should be left for the fragment header.
+ Note: buf.pos == buf.header means that the current fragment is empty and when
+ appending data to it, one byte should be left for the fragment header.
Invariant:
- (end - begin) is the number of bytes left to the end of block
+ (buf.end - buf.begin) is the number of bytes left to the end of block
*************************************************************************/
@@ -555,13 +555,13 @@ int close_current_fragment(backup_stream
INPUT BUFFER
Stores bytes read from input stream. These bytes are stored in the region
- starting from begin and ending at pos (pos points one byte after the end of
- data). The bytes starting from begin belong to the current fragment. The next
- fragment starts at byte pointed by header (this byte contains fragment`s
- header). When all data from the current fragment have been read, begin==header.
- To start reading next fragment, it must be entered so that header is moved to
- point at the header of the fragment following it (this is done in the
- load_next_fragment() function).
+ starting from buf.begin and ending at buf.pos (buf.pos points one byte after
+ the end of data). The bytes starting from buf.begin belong to the current
+ fragment. The next fragment starts at byte pointed by buf.header (this byte
+ contains fragment`s header). When all data from the current fragment have been
+ read, buf.begin == buf.header. To start reading next fragment, it must be
+ entered so that header is moved to point at the header of the fragment
+ following it (this is done in the load_next_fragment() function).
The header of next fragment can be inside the input buffer:
@@ -569,18 +569,18 @@ int close_current_fragment(backup_stream
next fragment (still in the stream)
[==========|XX:============].......................|
^ ^ ^ ^
- begin header pos end
+ buf.begin buf.header buf.pos buf.end
or still in the stream:
[=============]...........|........................|
^ ^ ^ ^
- begin pos header end
+ buf.begin buf.pos buf.header buf.end
- In each case header points at the position where the header should be if
- the data were present in the buffer. End points one byte after the end of
- current input block. As with header, this can be inside the buffer (end <= pos)
- or still in the stream.
+ In each case buf.header points at the position where the header should be if
+ the data were present in the buffer. Buf.end points one byte after the end of
+ current input block. As with header, this can be inside the buffer
+ (buf.end <= buf.pos) or still in the stream.
*************************************************************************/
@@ -788,13 +788,23 @@ int load_buffer(backup_stream *s)
/**
Setup input buffer to read next fragment.
+ Given that all bytes from the current fragment have been consumed,
+ enter next fragment and make it current.
+
This moves buf.header so that it points at the header of the fragment
following the fragment which is entered now. buf.begin will point at the
first byte of the entered fragment.
+
+ @pre All bytes from the current fragment have been consumed
+ (buf.begin == buf.header). The header byte is in the input buffer.
- @pre All bytes from previous fragment have been consumed and header of
- the next fragment is loaded into the buffer.
+ @post buf.begin points at first byte of the entered fragment; buf.head
+ indicates position of the header of the fragment following it.
+ @note The entered fragment contains buf.header - buf.begin bytes. It can
+ happen that buf.begin==buf.header (i.e., no bytes in the fragment) in the
+ case that the entered fragment consists of a single EOC marker.
+
@retval BSTREAM_OK next fragment has been entered
@retval BSTREAM_EOC the entered fragment is the last fragment of a chunk
@retval BSTREAM_EOS there are no more fragments in the stream
@@ -852,7 +862,7 @@ int load_next_fragment(backup_stream *s)
case BSTREAM_EOC: s->state= LAST_FRAGMENT; return BSTREAM_EOC;
- case FR_LAST: s->state= LAST_FRAGMENT;
+ case FR_LAST: s->state= LAST_FRAGMENT; return BSTREAM_EOC;
default: return BSTREAM_OK;
}
@@ -1393,7 +1403,6 @@ static
int check_end(backup_stream *s)
{
int ret;
- bool eoc_seen= FALSE;
if (s->state == ERROR)
return BSTREAM_ERROR;
@@ -1434,7 +1443,7 @@ int check_end(backup_stream *s)
if (*(s->buf.begin) == FR_EOC)
{
- eoc_seen= TRUE;
+ s->state= LAST_FRAGMENT;
s->buf.begin++;
s->buf.header++;
}
@@ -1450,10 +1459,7 @@ int check_end(backup_stream *s)
ASSERT(*(s->buf.begin) != FR_EOS);
ASSERT(*(s->buf.begin) != FR_EOC);
- if (eoc_seen || (s->state == LAST_FRAGMENT))
- return BSTREAM_EOC;
-
- return BSTREAM_OK;
+ return s->state == LAST_FRAGMENT ? BSTREAM_EOC : BSTREAM_OK;
}
/**
@@ -1537,7 +1543,12 @@ int bstream_read_part(backup_stream *s,
ASSERT(s->buf.pos > s->buf.header);
ret= load_next_fragment(s);
- if (ret != BSTREAM_OK)
+ /*
+ BSTREAM_OK and BSTREAM_EOC replies from load_next_fragment() mean
+ that the the next fragment has been successfully entered and we can
+ continue. Otherwise we quit here since we hit either ERROR or EOS.
+ */
+ if (ret != BSTREAM_OK && ret != BSTREAM_EOC)
return ret;
}
@@ -1549,9 +1560,9 @@ int bstream_read_part(backup_stream *s,
(buf.header <= buf.pos) - in this case all the remaining bytes of the
current fragment are in the buffer between buf.begin and buf.header,
- b) the header of the next framgment was not yet loaded into the input buffer
- (buf.pos < buf.header) - inis case all bytes in the buffer belong to the
- current fragment.
+ b) the header of the next fragment was not yet loaded into the input buffer
+ (buf.pos < buf.header) - in this case all bytes in the buffer belong to
+ the current fragment.
(See INPUT BUFFER description above)
*/
@@ -1567,7 +1578,7 @@ int bstream_read_part(backup_stream *s,
if (howmuch > 0)
{
/*
- Adjust howmuch if there is more data in the input bufer that would fit
+ Adjust howmuch if there is more data in the input buffer that would fit
into the blob. Copy only as much bytes as fits.
*/
if ((data->begin + howmuch) > data->end)
@@ -1731,6 +1742,7 @@ int bstream_read_blob(backup_stream *s,
int bstream_next_chunk(backup_stream *s)
{
int ret;
+ int more_fragments;
unsigned long int howmuch;
ASSERT(s->mode == READING);
@@ -1744,71 +1756,76 @@ int bstream_next_chunk(backup_stream *s)
IBUF_INV(s->buf);
/*
- If we are at the end of input block, load next one.
+ In the following loop we skip fragments until the last fragment of the
+ current chunk is reached.
*/
- if (s->buf.begin == s->buf.end)
- {
- ret= load_buffer(s);
- if (ret != BSTREAM_OK)
- return ret;
- }
+ do {
+ /*
+ When last fragment of the current chunk is entered in load_next_fragment()
+ then s->state is set to LAST_FRAGMENT.
+ */
+ more_fragments= (s->state == NORMAL);
- /* now we should be inside the current input block */
- ASSERT(s->buf.begin < s->buf.end);
+ /* if we are not at the end of the current fragment, move there */
+ if (s->buf.begin < s->buf.header)
+ {
+ /*
+ The end of the current fragment can be in the buffer, or still in
+ the stream. In the former case we just move the beginning of the buffer
+ to the end of the fragment, otherwise we move the stream forward and
+ empty the buffer so that the next fragment will be loaded into it when
+ the buffer is re-filled below.
+ */
+ if (s->buf.header < s->buf.pos)
+ s->buf.begin= s->buf.header;
+ else
+ {
+ howmuch= s->buf.header - s->buf.pos;
+ ret= as_forward(&s->stream, &howmuch);
+ if (ret != BSTREAM_OK)
+ return SERROR(s);
+ s->buf.begin= s->buf.pos= s->buf.header;
+ }
+ }
- /* if we are not at the beginning of next fragment, move there */
- if (s->buf.begin < s->buf.header)
- {
/*
- The header of next fragment can be in the buffer, or still in
- the stream. In former case we just move beginning of the buffer to the
- header, otherwise we move the stream forward and empty the buffer so that
- header will be loaded into it When the buffer is filled below.
- */
- if (s->buf.header < s->buf.pos)
- s->buf.begin= s->buf.header;
- else
+ If we are at the end of input block, load next one. Also, load bytes
+ into the buffer if it is empty.
+ */
+ if (s->buf.begin == s->buf.end || s->buf.pos == s->buf.begin)
{
- howmuch= s->buf.header - s->buf.pos;
- ret= as_forward(&s->stream, &howmuch);
+ ret= load_buffer(s);
if (ret != BSTREAM_OK)
- return SERROR(s);
-
- s->buf.begin= s->buf.pos= s->buf.header;
+ return ret;
}
- }
- /*
- Check that we are still inside the current input block ans at the beginnig
- of the fragment
- */
- ASSERT(s->buf.begin <= s->buf.end);
- ASSERT(s->buf.begin == s->buf.header);
+ /*
+ Check that we are inside the current input block, at the end
+ of the current fragment and that the input buffer is not empty.
+ */
+ ASSERT(s->buf.begin < s->buf.end);
+ ASSERT(s->buf.begin == s->buf.header);
+ ASSERT(s->buf.header < s->buf.pos);
- /*
- If buffer is empty, we load few bytes into it to have access to
- the fragment header.
- */
- if (s->buf.pos == s->buf.begin)
- {
- ret= load_buffer(s);
- if (ret != BSTREAM_OK)
- return ret;
- }
+ /*
+ Enter next fragment. If the fragment we are leaving now was not the last
+ fragment of the current chunk (more_fragments is true) then the loop will
+ continue, moving to the end of the new fragment and possibly skipping
+ more fragments of the current chunk.
+ */
+ ret= load_next_fragment(s);
+ if (ret == BSTREAM_ERROR)
+ return BSTREAM_ERROR;
- /* now we should have some bytes in the buffer */
- ASSERT(s->buf.pos > s->buf.header);
+ IBUF_INV(s->buf);
- ret= load_next_fragment(s);
- if (ret == BSTREAM_ERROR) /* To avoid invariant check below. */
- return BSTREAM_ERROR;
- /* if we hit EOC marker here, we treat it as empty chunk */
- if (ret == BSTREAM_EOC)
- ret= BSTREAM_OK;
+ if (ret == BSTREAM_EOS)
+ return BSTREAM_EOS;
- IBUF_INV(s->buf);
+ } while (more_fragments);
- return ret;
+ /* check for EOC or EOS markers */
+ return check_end(s);
}
/**
Attachment: [text/bzr-bundle] bzr/rafal.somla@sun.com-20090626062820-af8l8mkfcajjdwof.bundle
| Thread |
|---|
| • bzr commit into mysql-6.0-backup branch (Rafal.Somla:2833) Bug#45093 | Rafal Somla | 26 Jun |
