Magnus,
This is comments on several commits, ok ?
> 2929 Magnus Blåudd 2009-06-24
> ndbinfo
> - remove parameter apiTxnId from DbinfoScanRef
1) I think we should keep the apiTxnId (and maybe make it 64-bit like ordinary
transactions)
This cause I'm willing to make a bet that there will be a case if which one can not be
sure
if the signal is "still valid" (after failures/aborts)...and one need the txn-id to
determine.
I'm not 100% sure, and I haven't tried to construct a case, but in other protocols
there places
where such problems occur.
> 2925 Magnus Blåudd 2009-06-24
> ndbinfo
> - make DbinfoScanReq a class not struct to keep track of users
> - remove the dubious union construct for cursor part
2a) we prefer to keep the signal-data-classes structs, to verbosely express that they must
be POD
and to express that they are data-containers, not classes.
(but you can have the friend-declaration there anyway as documentation)
2b) i liked the dubious union construct, as it made sure that code is strict-aliasing
"safe"
> 2915 Magnus Blåudd 2009-06-23
> ndbinfo - part 2
> - add support for open and scan table
3) reading the code I see that "scan_nextreq" is sent already in execSCAN_CONF
this is have to change...
the current impl. means that a 10'' row table could be retreived wo/ user performing a
single nextResult()
"scan_nextreq" should only be sent when user calls nextResult()
/Jonas
Magnus Blåudd wrote:
> #At file:///home/msvensson/mysql/7.1-ndbinfo/ based on
> revid:magnus.blaudd@stripped
>
> 2929 Magnus Blåudd 2009-06-24
> ndbinfo
> - validate the tableId parameter when received in kernel and ndbapi
> - remove parameter apiTxnId from DbinfoScanRef
>
> modified:
> storage/ndb/include/kernel/signaldata/DbinfoScan.hpp
> storage/ndb/src/common/debugger/signaldata/DbinfoScan.cpp
> storage/ndb/src/kernel/blocks/LocalProxy.cpp
> storage/ndb/src/kernel/blocks/dbinfo/Dbinfo.cpp
> storage/ndb/src/ndbapi/NdbInfoScanOperation.cpp
> === modified file 'storage/ndb/include/kernel/signaldata/DbinfoScan.hpp'
> --- a/storage/ndb/include/kernel/signaldata/DbinfoScan.hpp 2009-06-24 21:05:37 +0000
> +++ b/storage/ndb/include/kernel/signaldata/DbinfoScan.hpp 2009-06-24 21:27:01 +0000
> @@ -134,10 +134,9 @@ class DbinfoScanRef {
> friend bool printDBINFO_SCAN_REF(FILE * output, const Uint32 * theData,
> Uint32 len, Uint16 receiverBlockNo);
>
> - STATIC_CONST( SignalLength = 3 );
> + STATIC_CONST( SignalLength = 2 );
>
> Uint32 tableId; // DBINFO table ID
> - Uint32 apiTxnId; // ID unique to API.
> Uint32 errorCode; // Error Code
> enum ErrorCode
> {
>
> === modified file 'storage/ndb/src/common/debugger/signaldata/DbinfoScan.cpp'
> --- a/storage/ndb/src/common/debugger/signaldata/DbinfoScan.cpp 2009-06-15 13:12:19
> +0000
> +++ b/storage/ndb/src/common/debugger/signaldata/DbinfoScan.cpp 2009-06-24 21:27:01
> +0000
> @@ -64,7 +64,6 @@ bool printDBINFO_SCAN_REF(FILE* output,
> {
> const DbinfoScanRef* sig = (const DbinfoScanRef*)theData;
> fprintf(output, " tableId: %u", sig->tableId);
> - fprintf(output, " apiTxnId: %u", sig->apiTxnId);
> fprintf(output, " errorCode: %u", sig->errorCode);
> fprintf(output, "\n");
> return true;
>
> === modified file 'storage/ndb/src/kernel/blocks/LocalProxy.cpp'
> --- a/storage/ndb/src/kernel/blocks/LocalProxy.cpp 2009-06-15 13:31:37 +0000
> +++ b/storage/ndb/src/kernel/blocks/LocalProxy.cpp 2009-06-24 21:27:01 +0000
> @@ -1033,7 +1033,6 @@ LocalProxy::sendDBINFO_SCANCONF(Signal*
> jam();
> DbinfoScanRef *ref = (DbinfoScanRef*)signal->getDataPtrSend();
> ref->tableId = conf->tableId;
> - ref->apiTxnId = conf->apiTxnId;
> ref->errorCode = ss.m_error;
> sendSignal(ss.m_req.senderRef, GSN_DBINFO_SCANREF, signal,
> DbinfoScanRef::SignalLength, JBB);
>
> === modified file 'storage/ndb/src/kernel/blocks/dbinfo/Dbinfo.cpp'
> --- a/storage/ndb/src/kernel/blocks/dbinfo/Dbinfo.cpp 2009-06-24 20:56:51 +0000
> +++ b/storage/ndb/src/kernel/blocks/dbinfo/Dbinfo.cpp 2009-06-24 21:27:01 +0000
> @@ -140,7 +140,7 @@ void Dbinfo::execDBINFO_SCANREQ(Signal *
> jamEntry();
> DbinfoScanReq req= *(DbinfoScanReq*)signal->theData;
>
> - const Uint32 tableId= req.tableId;
> + const Uint32 tableId= req.tableId;
> const Uint32 apiRef= req.apiRef;
> const Uint32 senderRef= req.senderRef;
> const Uint32 apiTxnId= req.apiTxnId;
> @@ -148,6 +148,18 @@ void Dbinfo::execDBINFO_SCANREQ(Signal *
> //const Uint32 colBitmapHi= req.colBitmapHi;
> //const Uint32 requestInfo= req.requestInfo;
>
> + // Validate tableId
> + if (tableId >= number_ndbinfo_tables)
> + {
> + jam();
> + DbinfoScanRef *ref= (DbinfoScanRef*)signal->getDataPtrSend();
> + ref->tableId= tableId;
> + ref->errorCode= DbinfoScanRef::NoTable;
> + sendSignal(senderRef, GSN_DBINFO_SCANREF, signal,
> + DbinfoScanRef::SignalLength, JBB);
> + return;
> + }
> +
> Uint32 i;
> int j;
> int startid= 0;
> @@ -272,18 +284,6 @@ void Dbinfo::execDBINFO_SCANREQ(Signal *
> default:
> jam();
>
> - if(tableId > number_ndbinfo_tables)
> - {
> - jam();
> - DbinfoScanRef *ref= (DbinfoScanRef*)signal->getDataPtrSend();
> - ref->tableId= tableId;
> - ref->apiTxnId= apiTxnId;
> - ref->errorCode= DbinfoScanRef::NoTable;
> - sendSignal(senderRef, GSN_DBINFO_SCANREF, signal,
> - DbinfoScanRef::SignalLength, JBB);
> - break;
> - }
> -
> ndbassert(tableId > 1);
>
> if(signal->getLength() == DbinfoScanReq::SignalLength)
> @@ -358,6 +358,9 @@ void Dbinfo::execDBINFO_SCANCONF(Signal
> //const Uint32 colBitmapLo= conf.colBitmapLo;
> //const Uint32 colBitmapHi= conf.colBitmapHi;
>
> + // Validate tableId
> + ndbassert(tableId < number_ndbinfo_tables);
> +
> DbinfoScanReq *oreq= (DbinfoScanReq*)signal->getDataPtrSend();
>
>
> memcpy(signal->getDataPtrSend(),&conf,signal->getLength()*sizeof(Uint32));
>
> === modified file 'storage/ndb/src/ndbapi/NdbInfoScanOperation.cpp'
> --- a/storage/ndb/src/ndbapi/NdbInfoScanOperation.cpp 2009-06-24 21:00:09 +0000
> +++ b/storage/ndb/src/ndbapi/NdbInfoScanOperation.cpp 2009-06-24 21:27:01 +0000
> @@ -297,6 +297,9 @@ NdbInfoScanOperation::execDBINFO_SCANCON
> const DbinfoScanConf* conf =
> CAST_CONSTPTR(DbinfoScanConf, sig->getDataPtr());
>
> + const Uint32 tableId = conf->tableId;
> + assert(tableId == m_table->getTableId());
> +
> if (conf->requestInfo & DbinfoScanConf::MoreData)
> {
> DBUG_PRINT("info", ("Request more data"));
> @@ -331,6 +334,9 @@ NdbInfoScanOperation::execDBINFO_SCANREF
> const DbinfoScanRef* ref =
> CAST_CONSTPTR(DbinfoScanRef, signal->getDataPtr());
>
> + const Uint32 tableId = ref->tableId;
> + assert(tableId == m_table->getTableId());
> +
> m_state = Error;
> DBUG_RETURN(ref->errorCode);
> }
>
>
>
> ------------------------------------------------------------------------
>
>
