[SFC-149] Race condition in test SfcIpv4PacketInHandlerTest.pktInPurgeBuffering Created: 16/Jun/16  Updated: 24/Jun/16  Resolved: 24/Jun/16

Status: Resolved
Project: sfc
Component/s: General
Affects Version/s: unspecified
Fix Version/s: None

Type: Bug
Reporter: Diego Granados Assignee: Diego Granados
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

Operating System: All
Platform: All


External issue ID: 6072

 Description   

There is a road condition in the mentioned test pckInPurgeBuffering that is making the test unstable (e.g. of one failed execution at https://jenkins.opendaylight.org/releng/job/sfc-verify-boron/jdk=openjdk8,nodes=dynamic_verify/166/console):

Tests run: 5, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.054 sec <<< FAILURE! - in org.opendaylight.sfc.ofrenderer.SfcIpv4PacketInHandlerTest
pktInPurgeBuffering(org.opendaylight.sfc.ofrenderer.SfcIpv4PacketInHandlerTest) Time elapsed: 0.02 sec <<< FAILURE!
org.mockito.exceptions.verification.WantedButNotInvoked:
Wanted but not invoked:
sfcOfFlowProgrammerImpl.setFlowRspId(<any>);
-> at org.opendaylight.sfc.ofrenderer.SfcIpv4PacketInHandlerTest.pktInPurgeBuffering(SfcIpv4PacketInHandlerTest.java:138)

This test verifies purging after timeout in the packetIn handler in charge of creating temporary rules in the PathMapperACL table. The problem: in the test, 1ms timeout is setted in the handler:

this.pktInHandler.setMaxBufferTime(1); // 1 microsecond

The comment is incorrect: time unit is not usecs, but millis (in packetInHandler, that value is compared vs System.currentTimeMillis for determining the purge):

if((currentMillis - bufferedTime.longValue()) > maxBufferTime)

Later in the test:

// When called again, the purgeCount will be exceeded, the buffer will
// be flushed, and this packet will be added back, making a size of 1
Thread.sleep(1); // sleep 1 millisecond, to let the buffer time expire

As stated, waiting exactly 1 milisecond (== exactly the timeout time) is the road condition. The test should be corrected in order to use a value higher enough (i.e. 10 millis) than the timeout so the buffer time is always exceeded. The incorrect "microseconds" comment should also be corrected



 Comments   
Comment by Diego Granados [ 16/Jun/16 ]

Just checked and the road condition is also present in pktInBufferingTimeout method. That should also be corrected, together with other comments uncorrectly commenting time units used in the test

Generated at Wed Feb 07 20:38:46 UTC 2024 using Jira 8.20.10#820010-sha1:ace47f9899e9ee25d7157d59aa17ab06aee30d3d.