When packet length is more than in buffered

[garysims]

First many thanks for GoMySQL, it is great... I am using it as part of my Go project "goesmtp" which is a multi-node ESMTP server written on Go (http://code.google.com/p/goesmtp/). I haven't released any code yet as it is still in its infancy but I am getting there!

Anyway during my development I have noticed a couple of situations where GoMySQL just froze. I have tracked down the problem to the getResult() function in mysql.go.

The problem is with these lines:

   // Check data length
   if int(hdr.length) > mysql.reader.Buffered() {
           mysql.error(CR_MALFORMED_PACKET, CR_MALFORMED_PACKET_STR)
           return
   }

What is happening is that when the results are quite large (large number of rows) from the MySQL DB there can be a situation where the hdr.length is greater than the current bytes in the buffer, but they are on the way (network latency I guess). Since this just returns it leaves everything in a very unstable state as getResult() is called again and again after this.

Anyway by just commenting out the lines everything works fine and the data is retrieved and handled correctly. I guess this isn't a real solution but maybe you have an idea about how to handle this better.

Another place where the same things happens is in the Query function. Here the code says:

if (mysql.reader.Buffered() == 0) {
    break
}

Which breaks out the the main loop processing packets... Again due to network latency Buffered can be 0 but there is more on the way. This results in large result sets never getting returned at the look break prematurely before the EOF is received.

To work around this I changed the code to:

    if ((mysql.curRes == nil) && (mysql.reader.Buffered() == 0)) {
        break
    }

Again, not sure if this is the best solution but works!

Thanks and best regards,

Gary

[Philio]

Hi Gary,

Thanks very much for the detailed explanation!

There are a few 'limitations' of the current release code that I've noticed and also realised I've made a few mistakes as I work more with the Go language. This was pretty much the first thing I ever wrote in Go :)

I'm working my way through the entire library and ironing out these bugs at the moment and this will be released as version 0.2. I'm not sure when this will be, but probably in a few days.

[garysims]

Phil,

Thanks for the quick reply... I look forward to seeing version 0.2

Thanks,

Gary

[Philio]

Hi Gary,

Just checked over this issue and you're absolutely right, just removing the check will be fine. As it's using bufio package it 'auto-fills' the buffer when the buffered bytes have been read. I could increase the buffer size, but I don't really think that is nescessary (plus will increase memory usage).

I'm still working on new release but won't be too much longer.

Phil.

[Philio]

This is resolved in 0.2.0 development version, currently in the master branch.

[garysims]

Phil,

Thanks for releasing version 0.2... the new error reporting looks great... but... unfortunately isn't doesn't 100% solve all my problems... sorry...

In my original post I mentioned a problem in the Query function:

if (mysql.reader.Buffered() == 0) {
    break
}

Which breaks out of the the main loop processing packets... Again due to network latency Buffered can be 0 but there is more on the way. This results in large result sets never getting returned at the look break prematurely before the EOF is received.

To work around this I changed the code to:

if ((mysql.curRes == nil) && (mysql.reader.Buffered() == 0)) {
    break
}

You see the problem isn't exactly as you imagine, you see the Buffered() function can return 0 even when there is more data on the way, all it is saying is that there is 0 in the buffer right now, it doesn't mean that the connection has ended...

So in this case the if statement tests as true as the Buffered function returns 0, but if you asked it again in a few milliseconds the result would be greater than 0. This is the danger of using the size of the buffer to determine if the data stream has ended.

The modified version of the code check to see if the EOF flag has been reached AND there is no more to read... The problem with this approach is that it will go badly wrong IF the packets are actually malformed...

My ESMTP server will only work reliably if I modify version 0.2 to include the extra check for mysql.curRes == nil.

Sorry to bring this up!

Gary PS. There is also a lurking check for hdr.length with Buffered() in the init().

[Philio]

Reopened this issue as not yet fully resolved!

[Philio]

Hi Gary,

I've commited a few changes this morning, diff here: http://github.com/Philio/GoMySQL/commit/9270aa956224ac985e1518163000fa05c13d0238

This should ensure that a complete result set is read, even if there is some kind of latency issue.

The only potential problem is that if there was a latency issue between a group of results obtained via MultiQuery, that the same thing could happen.

It's actually quite strange that you encountered this problem because for it to happen the following would have to happen:

MySQL sends result to client. Client receives exactly the data of 1 (or more) packet(s) of the result, but not all packets of the result. Client reads and processes the data of packet(s). Client sees an empty buffer and assumes EOF.

Let me know if there are any further problems with this fix.

[Philio]

I've tested this in depth tonight, including sending some fairly large packets of data backwards and forwards between client and server, for example this script inserts 100 x 1Mb blobs then selects and displays the results:

for i := 0; i < 100; i ++ {
    chars := "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890"
    data := make([]byte, 1024 * 1024)
    for j := 0; j < len(data); j ++ {
        rn := rand.Uint32()
        rp := rn % uint32(len(chars))
        data[j] = chars[rp]
    }
    str := string(data)
    db.Query("INSERT INTO `gotesting`.`test1` (`id` ,`a_bigint` ,`a_float` ,`a_varchar` ,`a_time` ,`a_timestamp` ,`a_blob`) VALUES (NULL , '1', '1.1', 'One', CURTIME(), CURRENT_TIMESTAMP, '" + str + "')") 
    if db.Errno != 0 {
        fmt.Printf("Error #%d %s\n", db.Errno, db.Error)
        os.Exit(1)
    }
}
res, _ := db.Query("SELECT * FROM test1")
if db.Errno != 0 {
    fmt.Printf("Error #%d %s\n", db.Errno, db.Error)
    os.Exit(1)
}
var row map[string] interface{}
for {
    row = res.FetchMap()
    if row == nil {
        break
    }
    for key, value := range row {
        fmt.Printf("%s:%v\n", key, value)
    }
}

I'm going to close this issue now, let me know if any further problems.

[garysims]

Phil,

Thanks for the great work... This looks like it has cleared up the problem... In my testing I saw over 5000 rows returned without any problems.

Many thanks,

Gary

[Philio]

Hi Gary

Good news, I also tested with a much larger dataset without issue (about 1Gb)