[TRAFODION-3183] fetch huge data give rise to core #1694
Conversation
|
Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2942/ |
|
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/2942/ |
| maxRowCnt = (int) (maxRowCnt / multi); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Server side is supposed to limit the size both at compile and run-time. Do you mean that limitation didn't kick in
There was a problem hiding this comment.
yes, server side not work well.
The fetch row count is sent from client side, so i think server side limition is not suitable
There was a problem hiding this comment.
In that case, similar issue will be seen with ODBC application and Type 2 driver applications.
It looks like the cqd MEMORY_LIMIT_ROWSET_IN_MB limits the rowset size during compile time, but doesn't limit the rowset size during run-time. It is possible to change the rowset size during run-time without specifying it during compile time. In that case, it would be good if we use the same CQD value to limit it during run-time too.
There was a problem hiding this comment.
MEMORY_LIMIT_ROWSET_IN_MB seems to use for insert don't for fetch
There was a problem hiding this comment.
Yes @mashengchen you are correct. I would think it is better to move this logic to server side if possible. Also, Is there a need to loop around till maxRowCont or EOF is reached. If not, I am ok with PR
There was a problem hiding this comment.
hi Selva , here is just change the maxRowCnt to a reasonable value. No need to loop, jdbc then will fetch rows according to the changed maxRowCnt
There was a problem hiding this comment.
Just spent some time looking at this change. As per JDBC specification, Statement.setMaxRows sets the upper limit on the number of rows in the result set. Statement.setFetchSize sets the hint for the driver to fetch how many rows are fetched in next(). maxRows_ and fetchSize_ in this method corresponds to these values respectively. It is possible that the application doesn't set this value at all. Any case, JDBC driver can decide the fetchSize based on the hint or its limitations. I would suggest the following:
- Move this logic to calculate the number of rows to getFetchSize
- Use a data source property to get the fetch size in terms of MB rather than assuming 1GB in calculation. Assume a default value a far less than 1GB says 50 or 100 MB if the property is not set.
- Calculate the number of rows fetchSize_ based on the setFetchSize and this property value which ever doesn't exceed the size in terms of MB.
- Use getFetchSize() in this method to get the fetchSize_
This would ensure that Trafodion conforms to JDBC specification in a better way because it would let the application know how the hint of setFetchSize works.
There was a problem hiding this comment.
BTW, sorry for not responding sooner
There was a problem hiding this comment.
thank you for the suggestion, aggress with point,1,3,4. for the second one, the 1GB limit currently is the max fetch size, if a larger value setted , it may cause mxosrvr core. also the purpose of this PR is to limit the fetch size, so I don't think it's need to give users the entance to set max fetch size.(what to do if users set it to 10GB), so i think only when fetch size larger than 1gb, we may do the limitation, to reduce the size to 1gb
There was a problem hiding this comment.
add note, there may have situations : one time fetch larger than 1GB when select hive datas or get lob data, at that time mxosrvr will down.
|
is there some one can review this? |
|
jenkins, retest |
|
@selvaganesang, does this look good to you now? |
|
Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/3071/ |
|
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/3071/ |
mashengchen
force-pushed
the
TRAFODION-3183
branch
from
bb68005
to
baa12a5
Compare
|
New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/3076/ |
|
Test Failed. https://jenkins.esgyn.com/job/Check-PR-master/3076/ |
|
New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/3077/ |
|
Test Failed. https://jenkins.esgyn.com/job/Check-PR-master/3077/ |
mashengchen
force-pushed
the
TRAFODION-3183
branch
from
903fdf8
to
939e0b8
Compare
|
New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/3078/ |
|
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/3078/ |
|
@selvaganesang do you think it's OK now |
|
is there someone can review this? |
|
I think @selvaganesang is still out on vacation; he should be back tomorrow. I will ping him then. |
mashengchen
force-pushed
the
TRAFODION-3183
branch
from
939e0b8
to
4bc54e8
Compare
|
New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/3079/ |
mashengchen
force-pushed
the
TRAFODION-3183
branch
from
4bc54e8
to
d1bc87a
Compare
|
Test Failed. https://jenkins.esgyn.com/job/Check-PR-master/3079/ |
|
Previous Test Aborted. New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/3080/ |
|
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/3080/ |
|
Looks ok with some subtle corrections needed. getFetchSize needs to call setFetchSizeIfExceedLimit() so that it can return correct value of fetch size when getFetchSize is issued without calling next(). You can do some optimization so that setFetchSizeIfExceededLimit is called only once instead of calculating the fetch size every time next() is called. FetchSize can be obtained via a data source property to control the size of the buffer to ship from server to client. Currently it is either 100 rows or 1 GB |
each time do fetch , calc whether fetch size is bigger than 1GB, if true ,devide fetch times depend on fetchsize/1gb