Openbravo Issue Tracking System - Retail Modules
View Issue Details
0037851Retail ModulesWeb POSpublic2018-02-07 13:222018-03-09 10:50
marvintm 
marvintm 
highmajoralways
closedfixed 
5
 
RR18Q2 
migueldejuana
No
0037851: Request timeout should apply to the whole process, and not just the query
Currently in some requests to ProcessHQLQuery masterdata classes, we receive a timeout value which we use when executing the query.

This is correct, but it's not enough: if the rest of the process (mainly, the reading of the returned data, generation of the JSON information, and writing of this JSON into the response) takes too long, the client side will already reach timeout, and all the work we do will be for nothing.

We can improve this behaviour, by measuring the time the request has already taken, and when reading the data and generating the response, checking the time and if it's gone pass the timeout, cancel the process.

This would be specially beneficial if for example some clients are having network problems, and are unable to receive the data in a reasonable amount of time. The fix would free resources in the server (bandwidth, database connections, threads) that would be otherwise consumed doing useless work, as the client has already abandoned the request because of the timeout.
This problem can be reproduced easily if a breakpoint is set after the query has already been executed but the response has not yet been computed.
Performance
related to defect 0038007 closed marvintm LogClient can generate too many useless requests if server is struggling 
related to design defect 0038036 closed mtaal SS in online mode always forward all requests to the CS 
related to design defect 0038390 closed marvintm request timeout is checked only after reading the whole request content 
Issue History
2018-02-07 13:22marvintmNew Issue
2018-02-07 13:22marvintmAssigned To => Retail
2018-02-07 13:22marvintmTriggers an Emergency Pack => No
2018-02-09 09:26alostaleTag Attached: Performance
2018-02-09 09:26alostaleIssue Monitored: alostale
2018-02-27 11:59alostaleRelationship addedrelated to 0038007
2018-02-27 13:36marvintmStatusnew => scheduled
2018-02-27 13:36marvintmAssigned ToRetail => marvintm
2018-02-28 11:31hgbotCheckin
2018-02-28 11:31hgbotNote Added: 0102866
2018-02-28 11:31hgbotStatusscheduled => resolved
2018-02-28 11:31hgbotResolutionopen => fixed
2018-02-28 11:31hgbotFixed in SCM revision => http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/f9924a8180ef5dbd7fc7dc711161fa0dc5443bc2 [^]
2018-02-28 11:31hgbotCheckin
2018-02-28 11:31hgbotNote Added: 0102868
2018-02-28 11:33marvintmReview Assigned To => migueldejuana
2018-02-28 17:14migueldejuanaNote Added: 0102874
2018-02-28 17:14migueldejuanaStatusresolved => closed
2018-03-02 14:55hgbotCheckin
2018-03-02 14:55hgbotNote Added: 0102936
2018-03-02 14:55hgbotCheckin
2018-03-02 14:55hgbotNote Added: 0102937
2018-03-02 14:55hgbotCheckin
2018-03-02 14:55hgbotNote Added: 0102938
2018-03-02 14:55hgbotCheckin
2018-03-02 14:55hgbotNote Added: 0102939
2018-03-02 14:59marvintmStatusclosed => new
2018-03-02 14:59marvintmResolutionfixed => open
2018-03-02 14:59marvintmStatusnew => scheduled
2018-03-02 14:59marvintmStatusscheduled => resolved
2018-03-02 14:59marvintmFixed in Version => RR18Q2
2018-03-02 14:59marvintmResolutionopen => fixed
2018-03-05 06:17mtaalRelationship addedblocks 0038036
2018-03-05 06:17mtaalRelationship deletedblocks 0038036
2018-03-05 06:17mtaalRelationship addedrelated to 0038036
2018-03-05 10:23migueldejuanaNote Added: 0102973
2018-03-05 10:23migueldejuanaStatusresolved => closed
2018-03-09 10:50hgbotCheckin
2018-03-09 10:50hgbotNote Added: 0103119
2018-03-09 10:50hgbotCheckin
2018-03-09 10:50hgbotNote Added: 0103120
2018-04-17 13:03alostaleRelationship addedrelated to 0038390

Notes
(0102866)
hgbot   
2018-02-28 11:31   
Repository: erp/pmods/org.openbravo.mobile.core
Changeset: f9924a8180ef5dbd7fc7dc711161fa0dc5443bc2
Author: Antonio Moreno <antonio.moreno <at> openbravo.com>
Date: Tue Feb 27 17:01:53 2018 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/f9924a8180ef5dbd7fc7dc711161fa0dc5443bc2 [^]

Fixed issue 37851. Several improvements to cancel requests which are no longer needed:
- Now a 'timeToTimeout' is computed for each SecuredJSONProcess that contains a Timeout in the request information. This timeToTimeout is the instant (in number of miliseconds) in which the timeout would be reached.
- A new API, hasTimeoutBeenReached(), has been created in the SecuredJSONProcess class, which then any process can use to find out if the timeout has been reached.
- For now, this API is used in the ProcessHQLQuery class to detect in several points during the process execution if timeout was reached, and if it was, the process is then canceled.
- Also, the timeout passed to the query executed in the database has been changed, and now it takes into account the time already spent in the rest of the request. This is important because if for example the request took a significant amount of time to process (due to low network bandwidth for example), we actually have less available time to execute the rest of the process, which includes the query.

---
M src/org/openbravo/mobile/core/process/MobileService.java
M src/org/openbravo/mobile/core/process/MobileServiceProcessor.java
M src/org/openbravo/mobile/core/process/ProcessHQLQuery.java
M src/org/openbravo/mobile/core/process/Scroll.java
M src/org/openbravo/mobile/core/process/SecuredJSONProcess.java
M src/org/openbravo/mobile/core/servercontroller/SynchronizedServerProcessCaller.java
---
(0102868)
hgbot   
2018-02-28 11:31   
Repository: erp/pmods/org.openbravo.mobile.core
Changeset: 50c01da42269d805fb2539f418d173d13152885d
Author: Antonio Moreno <antonio.moreno <at> openbravo.com>
Date: Wed Feb 28 09:18:48 2018 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/50c01da42269d805fb2539f418d173d13152885d [^]

Related to issue 37851. TimeToTimeout will not be set if it is null, to prevent NPE in ExternalOrderLoader.

---
M src/org/openbravo/mobile/core/process/MobileServiceProcessor.java
---
(0102874)
migueldejuana   
2018-02-28 17:14   
Tested and reviewed
(0102936)
hgbot   
2018-03-02 14:55   
Repository: erp/pmods/org.openbravo.mobile.core
Changeset: cf64dde23bcb2b226823aeceb15359283dc6a147
Author: Antonio Moreno <antonio.moreno <at> openbravo.com>
Date: Thu Mar 01 12:13:48 2018 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/cf64dde23bcb2b226823aeceb15359283dc6a147 [^]

Related to issue 37851. Several improvements have been made:
- Now initial time is measured from the moment the request starts, instead of being measured after the request content has already been read
- Now POST requests send the timeout also as part of the URL, and is read before, so that the process can be canceled while reading the request content if it takes too much time
- Improved log message information, and removed unneeded second stacktrace
- Now checkTimeout() has been moved to SecuredJSONProcess, and it's considered the API that all processes should use

---
M src/org/openbravo/mobile/core/process/MobileService.java
M src/org/openbravo/mobile/core/process/MobileServiceProcessor.java
M src/org/openbravo/mobile/core/process/ProcessHQLQuery.java
M src/org/openbravo/mobile/core/process/Scroll.java
M src/org/openbravo/mobile/core/process/SecuredJSONProcess.java
M src/org/openbravo/mobile/core/process/WebServiceAuthenticatedServlet.java
M src/org/openbravo/mobile/core/servercontroller/SynchronizedServerProcessCaller.java
M web/org.openbravo.mobile.core/source/data/ob-datasource.js
---
(0102937)
hgbot   
2018-03-02 14:55   
Repository: erp/pmods/org.openbravo.mobile.core
Changeset: e8b4bae0b38aa6bde1d5584433d2ae3c2e7e90f5
Author: Antonio Moreno <antonio.moreno <at> openbravo.com>
Date: Thu Mar 01 14:02:27 2018 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/e8b4bae0b38aa6bde1d5584433d2ae3c2e7e90f5 [^]

Related to issue 37851. Several further improvements:
- Added missing exception class
- Improved log message
- Changed visibility of some methods and properties
- Ensure that the response is the same if the timeout is reached while reading the request, or after that in some other point

---
M src/org/openbravo/mobile/core/process/MobileService.java
M src/org/openbravo/mobile/core/process/MobileServiceProcessor.java
M src/org/openbravo/mobile/core/process/ProcessHQLQuery.java
M src/org/openbravo/mobile/core/process/SecuredJSONProcess.java
M src/org/openbravo/mobile/core/process/WebServiceAuthenticatedServlet.java
A src/org/openbravo/mobile/core/process/RequestTimeoutException.java
---
(0102938)
hgbot   
2018-03-02 14:55   
Repository: erp/pmods/org.openbravo.mobile.core
Changeset: a54f6be3fabeaf3873886b7a51b1c2118cd49343
Author: Antonio Moreno <antonio.moreno <at> openbravo.com>
Date: Thu Mar 01 15:43:18 2018 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/a54f6be3fabeaf3873886b7a51b1c2118cd49343 [^]

Related to issue 37851. Fix javadoc. Avoid NPE if timeout is null

---
M src/org/openbravo/mobile/core/process/MobileServiceProcessor.java
M src/org/openbravo/mobile/core/process/SecuredJSONProcess.java
---
(0102939)
hgbot   
2018-03-02 14:55   
Repository: erp/pmods/org.openbravo.mobile.core
Changeset: 6986cf69c6cedf2495828efdddc71561c74bcd9f
Author: Antonio Moreno <antonio.moreno <at> openbravo.com>
Date: Fri Mar 02 13:03:55 2018 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/6986cf69c6cedf2495828efdddc71561c74bcd9f [^]

Related to issue 37851. Changed non-synced models popup to take into account the addition of the timeout parameter in the URL

---
M web/org.openbravo.mobile.core/source/component/dialog/ob-modalmodelstosync.js
---
(0102973)
migueldejuana   
2018-03-05 10:23   
Tested and reviewed
(0103119)
hgbot   
2018-03-09 10:50   
Repository: erp/pmods/org.openbravo.mobile.core
Changeset: 97006e7169be279e88224834b03eb4ae91d6ee98
Author: Gorka Gil <gorka.gil <at> openbravo.com>
Date: Thu Mar 08 19:24:46 2018 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/97006e7169be279e88224834b03eb4ae91d6ee98 [^]

Related to issue 37851: improve logged info

- Add more info to the log of timeout errors:
  - Remote ip
  - Initial date of the request
- Fix the way to show failing class in MobileService.java
- Time out when reading: show read and total size of the request
- Show write lines in the response

---
M src/org/openbravo/mobile/core/process/MobileService.java
M src/org/openbravo/mobile/core/process/MobileServiceProcessor.java
M src/org/openbravo/mobile/core/process/ProcessHQLQuery.java
M src/org/openbravo/mobile/core/process/Scroll.java
M src/org/openbravo/mobile/core/process/SecuredJSONProcess.java
M src/org/openbravo/mobile/core/process/WebServiceAuthenticatedServlet.java
M src/org/openbravo/mobile/core/servercontroller/SynchronizedServerProcessCaller.java
A src/org/openbravo/mobile/core/process/RequestTimeoutWithMessageException.java
---
(0103120)
hgbot   
2018-03-09 10:50   
Repository: erp/pmods/org.openbravo.retail.posterminal
Changeset: f51ed7d772a49275c8a094754800d01a27720391
Author: Gorka Gil <gorka.gil <at> openbravo.com>
Date: Thu Mar 08 19:27:04 2018 +0100
URL: http://code.openbravo.com/erp/pmods/org.openbravo.retail.posterminal/rev/f51ed7d772a49275c8a094754800d01a27720391 [^]

Related to issue 37851: Improve logged info

Add specific timeout message for Terminal class:

Capturing the scroll timeout message that contains the index of the page in which has give timeout,
and realaunching a normal time out exception adding the rest of information.

---
M src/org/openbravo/retail/posterminal/term/Terminal.java
---