sunhelly commented on a change in pull request #4249:
URL: https://github.com/apache/hbase/pull/4249#discussion_r832782056
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
##########
@@ -3727,14 +3734,24 @@ public ScanResponse scan(final RpcController
controller, final ScanRequest reque
if (context != null) {
context.setCallBack(rsh.shippedCallback);
} else {
- // When context != null, adding back the lease will be done in
callback set above.
- addScannerLeaseBack(lease);
+ // If context is null,we call rsh.shippedCallback directly to
release the
+ // internal resources in rsh.
+ runShippedCallback(rsh);
Review comment:
The issue here is scanner closed too early then the result bytebuffer
might be overwritten. I think the scanner resources are released and lease is
added back too, so is it necessary to change codes here?And if there is no rpc
call context here, is it a little strange to run a kind of RpcCallBack?
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
##########
@@ -3727,14 +3734,24 @@ public ScanResponse scan(final RpcController
controller, final ScanRequest reque
if (context != null) {
context.setCallBack(rsh.shippedCallback);
} else {
- // When context != null, adding back the lease will be done in
callback set above.
- addScannerLeaseBack(lease);
+ // If context is null,we call rsh.shippedCallback directly to
release the
+ // internal resources in rsh.
+ runShippedCallback(rsh);
Review comment:
The issue here is scanner closed too early then the result bytebuffer
might be overwritten. I think the scanner resources are released and lease is
added back too, so I doubt if it is necessary to change codes here. And if
there is no rpc call context here, is it a little strange to run a kind of
RpcCallBack?
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
##########
@@ -7871,8 +7869,8 @@ void prepareGet(final Get get) throws IOException {
// This can be an EXPENSIVE call. It may make an extra copy from offheap
to onheap buffers.
// See more details in HBASE-26036.
for (Cell cell : tmp) {
- results.add(cell instanceof ByteBufferExtendedCell ?
- KeyValueUtil.copyToNewKeyValue(cell) : cell);
+ results.add(
+ CellUtil.cloneIfNecessary(cell));
Review comment:
one line is ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
