[AGENT++] AgentX++ suagent commiterror let the RowStatus in detroy(6); the row is block forever!

[Frank Fock]

Hi Claus,

As the documentation of the AgentXSharedTable points out,
the allocate_index method has to be called manually
and it is NOT called by add_row.

The question you probably want to ask is: "Why"?

A shared table does not make sense, if you have a single
subagent only (as in your test).
If there are more than one agent, a row creation through
RowStatus mechanism, cannot work, because for the new
row, it is unclear which subagent should allocate/register
the row.

That is the reason, why the allocate_index has not been
called automatically by add_row (it won't work in real-life
agents).

Instead, the agentppTestSessionsTable exists that contains
a row for each subagent session. It provides the means needed
to be able to create a row in the associated shared table
on behalf of an user selected subagent.

Consequently, the problem with the agentppTestSharedTable
implementation is that it allows the createAndWait and
createAndGo operations (without proper index allocation).

It is not valid to deduce from that table implementation
on problems with other shared tables.

I will fix the agentppTestSharedTable implementation
as indicated by the [APP-14] issue.

Best regards,
Frank


On 02.11.2010 22:40, Claus Klein wrote:
> Hallo Frank,
>
> I really need help.
>
> If you study the log at the end of my scripts you will see the there is
> an agentx error while destroy.
>
> The reason is the deallocate_index() is called while remove_row(), but
> it was never allocated in add_row().
> How should this shared table really work?
>
> Fact is that net-snmp does not accept this deallocate_index.
> Last but not least, still with this uncommented version there is a
> genError while rowdestroy happens?
>
> Claus
>
> ./agentx_usecase_rowstatus2.sh 2
>
> #Setup
> + printf '\n#Pre-condition: row must not exist!\n'
>
> #Pre-condition: row must not exist!
> + '[' 2 -gt 1 ']'
> + snmpget -v3 -r0 -t 5 -uunsecureUser localhost:4700
> AGENTPP-TEST-MIB::agentppTestSharedTableRowStatus.2
> + grep -v -q 'No Such Instance'
> + printf '\n#Test case: create non-existing table with RowStatus with 3
> shots\n'
>
> #Test case: create non-existing table with RowStatus with 3 shots
> + snmpget -v3 -r0 -t 5 -uunsecureUser localhost:4700
> AGENTPP-TEST-MIB::agentppTestSharedTableRowStatus.2
> + grep -q 'No Such Instance'
> + snmpset -v3 -r0 -t 5 -uunsecureUser localhost:4700
> AGENTPP-TEST-MIB::agentppTestSharedTableRowStatus.2 = createAndWait
> AGENTPP-TEST-MIB::agentppTestSharedTableRowStatus.2 = INTEGER:
> createAndWait(5)
> + printf '\n#Condition: row must now exist!\n'
>
> #Condition: row must now exist!
> + snmpget -v3 -r0 -t 5 -uunsecureUser localhost:4700
> AGENTPP-TEST-MIB::agentppTestSharedTableRowStatus.2
> + grep -v -q 'No Such Instance'
> + snmpset -v3 -r0 -t 5 -uunsecureUser localhost:4700
> AGENTPP-TEST-MIB::agentppTestSharedTableRowStatus.2 = notInService
> AGENTPP-TEST-MIB::agentppTestSharedTableDelay.2 = 2
> AGENTPP-TEST-MIB::agentppTestTimeout.0 = 2
> AGENTPP-TEST-MIB::agentppTestSharedTableRowStatus.2 = INTEGER:
> notInService(2)
> AGENTPP-TEST-MIB::agentppTestSharedTableDelay.2 = INTEGER: 2 1/100 seconds.
> AGENTPP-TEST-MIB::agentppTestTimeout.0 = Gauge32: 2 1/1000 seconds.
> + snmpset -v3 -r0 -t 5 -uunsecureUser localhost:4700
> AGENTPP-TEST-MIB::agentppTestSharedTableRowStatus.2 = active
> AGENTPP-TEST-MIB::agentppTestSharedTableRowStatus.2 = INTEGER: active(1)
> + printf '\n#Post-condition: row must exist and in state active!\n'
>
> #Post-condition: row must exist and in state active!
> + snmpget -v3 -r0 -t 5 -uunsecureUser localhost:4700
> AGENTPP-TEST-MIB::agentppTestSharedTableRowStatus.2
> + grep -qw active
> + printf '\n#Cleanup\n'
>
> #Cleanup
> + snmpset -v3 -r0 -t 5 -uunsecureUser localhost:4700
> AGENTPP-TEST-MIB::agentppTestSharedTableRowStatus.2 = destroy
> Error in packet.
> Reason: (genError) A general failure occured
>
>
> _________________________________________________
>
> MibTableRow* AgentXSharedTable::add_row(const Oidx& ind)
> {
> if ((ind.len() == 0) || (generator.size() == 0)) return 0;
>
> // check whether row exists
> MibTableRow* new_row = find_index(ind);
> boolean existing = (new_row != 0);
> if (!new_row) {
> new_row = new MibTableRow(generator);
> new_row->set_index(ind);
> }
> // register row
> Oidx subtree(new_row->get_nth(0)->get_oid());
> unsigned int u = (*(generator.last()->key()))[0];
> AgentXRegion r(subtree, (unsigned char)(key()->len()+1), u);
> backReference->register_region(myContext, r, timeout, TRUE, this);
> //below event is fired when confirmation of registration is received
> //fire_row_changed(rowCreateAndGo, new_row, ind);
> if (existing)
> return new_row;
> return content.add(new_row);
> }
>
> void AgentXSharedTable::remove_row(const Oidx& ind)
> {
> Oidx o(ind);
> MibTableRow* r = content.find(&o);
>
> #if 0 // FIXME: never allocate_index() called! see add_row() ck
> // deallocate index
> Vbx* vbs = create_index_vbs(ind);
> if (vbs) {
> backReference->deallocate_index(myContext, vbs, index_len, this);
> delete[] vbs;
> }
> #endif
>
> // unregister row
> Oidx subtree(r->get_nth(0)->get_oid());
> unsigned int u = r->last()->get_oid()[key()->len()];
> AgentXRegion reg(subtree, (unsigned char)(key()->len()+1), u);
> backReference->register_region(myContext, reg, 0, FALSE);
>
> fire_row_changed(rowDestroy, r, ind);
>
> if (r) notready_rows.remove(r);
> content.remove(&o);
> }
>
> On 31.10.2010, at 12:02, Claus Klein wrote:
>
>> Hi Frank,
>>
>> my first interpretation of my test was not right. The handling with
>> commit failed works fine!
>>
>> But still there may be a problem if the AgentXSlave 'deallocate
>> request' fails.
>> Too, It is not very efficiently if we register a row only to destroy
>> it immediately?
>>
>>
>> I changed a little bit at the code and improved the RowStatus destroy
>> handling.
>> I hope it is right, I am not sure! Please check it.
>>
>> Too I improved my test scripts, I attache my patch and the scripts.
>>
>> With regards,
>>
>> Claus
>>
>> claus-kleins-macbook-pro:AgentPro clausklein$ bzr diff -p1
>> === modified file 'agent++/src/mib.cpp'
>> --- old/agent++/src/mib.cpp 2010-10-31 08:30:19 +0000
>> +++ new/agent++/src/mib.cpp 2010-10-31 10:28:54 +0000
>> @@ -572,10 +572,10 @@
>> return ((l == rowNotInService) ||
>> (l == rowActive) || (l == rowDestroy));
>> default:
>> - return FALSE;
>> + return (l == rowDestroy); // FIXME: added! We have to allow destroy
>> in each state! ck
>> }
>> }
>> - else
>> + else // FIXME: what state is this? ck
>> return ((l == rowCreateAndGo) || (l == rowCreateAndWait) ||
>> (l == rowDestroy));
>> }
>> @@ -672,7 +672,7 @@
>> break;
>> case rowDestroy:
>> set_value(rs);
>> - delete undo; // No undo of row destroy!
>> + delete undo; // FIXME: why, ck? No undo of row destroy!
>> undo = 0;
>> break;
>> default:
>> @@ -2177,10 +2177,12 @@
>> ok = TRUE;
>> wait = TRUE;
>> break;
>> +#if 1 // FIXME: Do we allow destroy of a nonexiting row without
>> error? YES! ck
>> case rowDestroy:
>> ok = TRUE;
>> ignore = TRUE;
>> break;
>> +#endif
>> default:
>> delete[] fulfilled;
>> delete[] required;
>> @@ -2236,6 +2238,7 @@
>> pvbs[col] = req->get_value(i);
>> }
>> }
>> +
>> // test values through prepare
>> MibTableRow* new_row = new MibTableRow(generator);
>> new_row->set_index(new_index);
>> @@ -2304,14 +2307,15 @@
>> else
>> voting_result = perform_voting(new_row,
>> rowEmpty, rowDestroy);
>> - delete new_row;
>> -
>> // Unset the ready flag of the current column.
>> // via this one all the other columns are set by commit_set_request
>> req->unset_ready(ind);
>> delete[] fulfilled;
>> delete[] required;
>> delete[] pvbs;
>> + delete new_row; // FIXME: same oder as other cleanup blocks; ck
>> + // TODO: use a macro for cleanup or better spart pointers? ck
>> +
>> return voting_result;
>> }
>>
>>
>> claus-kleins-macbook-pro:AgentPro clausklein$
>>
>>
>>
>> On 29.10.2010, at 22:59, Claus Klein wrote:
>>
>>> Hi Frank,
>>>
>>> I have found a third issue about the RowStatus handling at the subagent.
>>> With this script, you can force this error as many times you want,
>>> simple choose a new row index.
>>>
>>> Kindly regards,
>>> Claus
>>>
>>
>> _______________________________________________
>> AGENTPP mailing list
>> AGENTPP at agentpp.org
>> http://lists.agentpp.org/mailman/listinfo/agentpp
>

-- 
AGENT++
http://www.agentpp.com
http://www.snmp4j.com
http://www.mibexplorer.com
http://www.mibdesigner.com