[NETCONF-514] Spurious reads before put in 8040 implementation across a mounted resource Created: 23/Feb/18 Updated: 28/Feb/18 Resolved: 28/Feb/18 |
|
| Status: | Resolved |
| Project: | netconf |
| Component/s: | restconf-nb |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Low |
| Reporter: | Anton Ivanov | Assignee: | Unassigned |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Environment: |
Observed with JSON RPC, should be observable using any mounted data resource broker. |
||
| Description |
|
8040 code does unnecessary reads when a PUT is given via restconf. A PUT according to RFC is a "create or modify". There is absolutely no need to perform a read here. This is not a POST where ODL needs to verify if the resource exists beforehand. This is purely a performance penalty issue, it does not effect functionality |
| Comments |
| Comment by Tom Pantelis [ 25/Feb/18 ] |
|
The read is done to satisfy RFC8040 section 4.5 (https://tools.ietf.org/html/rfc8040#section-4.5): "Consistent with [RFC7231], if the PUT request creates a new resource, a "201 Created" status-line is returned. If an existing resource is modified, a "204 No Content" status-line is returned.". So the read result is used to initialize the response status in ResponseFactory.prepareStatus: return readData != null ? Status.OK : Status.CREATED; However, according to the RFC, Status.OK should be Status.NO_CONTENT. |
| Comment by Anton Ivanov [ 26/Feb/18 ] |
|
This makes more sense. However, in that case it may be worth it to consider using exist() - this is the same cost internally and for mount points it does not incur the penalty of transferring all the data under this node. |
| Comment by Tom Pantelis [ 26/Feb/18 ] |
|
Agreed. Also the exists call can be done in parallel with the submit. |
| Comment by Tom Pantelis [ 26/Feb/18 ] |
|
Actually doing it in parallel when accessing the data store would be OK but we can't be sure about mount points. |
| Comment by Tom Pantelis [ 26/Feb/18 ] |