HPCC-11522 libmemcached plugin #6691
Conversation
|
https://track.hpccsystems.com/browse/HPCC-11522 |
|
@ghalliday re-targeted. |
jamienoss
force-pushed
the
issue11522-support-memcached-b4-pool-removed
branch
3 times, most recently
from
3c1ce38
to
68277af
Compare
| @@ -81,11 +81,12 @@ IF ("${COMMONSETUP_DONE}" STREQUAL "") | |||
| option(USE_PYTHON "Enable Python support" ON) | |||
| option(USE_V8 "Enable V8 JavaScript support" ON) | |||
| option(USE_JNI "Enable Java JNI support" ON) | |||
| option(USE_RINSIDE "Enable R support" ON) | |||
| option(USE_RINSIDE "Enable R support support" ON) | |||
jamienoss
force-pushed
the
issue11522-support-memcached-b4-pool-removed
branch
from
68277af
to
21e84ca
Compare
| msg.append(key).append("' is of type ").append(enumToStr((eclDataType)(flag))).append(", not ").append(enumToStr(eclType)).append(" as requested.\n"); | ||
|
|
||
| if (++typeMismatchCount <= MAX_TYPEMISMATCHCOUNT) | ||
| ctx->addWuException(msg.str(), WRN_FROM_PLUGIN, ExceptionSeverityInformation, ""); |
There was a problem hiding this comment.
I'm not convinced by this. I imagined it would report a warning N times, and then not report any more. This will keep reporting warnings/errors but will still continue executing. I think the else branch just needs to go.
There was a problem hiding this comment.
Doesn't ExceptionSeverityError terminate exec? This aside, I'll remove that branch.
There was a problem hiding this comment.
no. It just adds it into a different category
There was a problem hiding this comment.
Is there one that does then?
|
@jamienoss getting closer. |
|
@ghalliday like standing at conformal infinity and watching someone pass an event horizon! |
jamienoss
force-pushed
the
issue11522-support-memcached-b4-pool-removed
branch
from
fece0a7
to
a5f4765
Compare
|
@ghalliday back to you. |
| CriticalBlock block(crit); | ||
| if (!cachedConnection) | ||
| { | ||
| cachedConnection.set(new MemCachedPlugin::MCached(ctx, servers)); |
There was a problem hiding this comment.
This should be .setown, same for the call below. It will leak as it stands.
|
@jamienoss still some problems. |
|
@ghalliday any better? |
|
|
||
| if (sizeof(type)!=returnLength) | ||
| { | ||
| VStringBuffer msg("MemCachedPlugin: ERROR - Requested type (%luB) of different size from that stored (%luB). Check logs for more information.", sizeof(type), returnLength); |
There was a problem hiding this comment.
Unfortunately has a couple of problems:
- You can't assume sizeof is 64bit/fixed size. So whenever you pass sizeof/size_t to printf you need to cast it to a fixed size type. (It does affect the stack layout and will give you wrong values on some machines). In this case I would cast to unsigned since they're small.
- %lu is used for a long integer. This is different from a 64 bit integer. Because the format specifiers for 64bit differ between windows and linux you would need to say.
...(%"I64F"u...
if the argument is 64bit.
There was a problem hiding this comment.
picky: the size should come after "size" rather than "type"
|
@jamienoss close but... |
|
@richardkchapman Cheers for the intel. VStringBuffer is much neater. |
Previously: hpcc-systems#6368 & hpcc-systems#6686 Signed-off-by: jamienoss <james.noss@lexisnexis.com>
Signed-off-by: jamienoss <james.noss@lexisnexis.com>
Signed-off-by: jamienoss <james.noss@lexisnexis.com>
jamienoss
force-pushed
the
issue11522-support-memcached-b4-pool-removed
branch
from
9beb378
to
f482eef
Compare
|
{Miracle occurs} |
…b4-pool-removed HPCC-11522 libmemcached plugin Reviewed-by: Gavin Halliday <ghalliday@hpccsystems.com>
Previously: #6368 & #6686
Signed-off-by: jamienoss james.noss@lexisnexis.com