[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 |
||
| 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 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 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 |