sandeepvinayak commented on a change in pull request #2556:
URL: https://github.com/apache/hbase/pull/2556#discussion_r510491746
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALPrettyPrinter.java
##########
@@ -301,15 +318,12 @@ public void processFile(final Configuration conf, final
Path p)
List<Map<String, Object>> actions = new ArrayList<>();
for (Cell cell : edit.getCells()) {
// add atomic operation to txn
- Map<String, Object> op = new HashMap<>(toStringMap(cell,
outputOnlyRowKey));
- if (outputValues) {
- op.put("value", Bytes.toStringBinary(CellUtil.cloneValue(cell)));
- }
- // check row output filter
- if (row == null || ((String) op.get("row")).equals(row)) {
Review comment:
I see what you mean, I changed the flow because it was not very readable
even though it was functionally correct. Here is why I thought so:
older version
```java
Map<String, Object> op = new HashMap<>(toStringMap(cell,
outputOnlyRowKey));
if (outputValues) {
op.put("value", Bytes.toStringBinary(CellUtil.cloneValue(cell)));
}
// check row output filter
if (row == null || ((String) op.get("row")).equals(row)) {
actions.add(op);
}
op.put("total_size_sum", cell.heapSize());
```
Here, we are computing the stringMap and then checking if we need this row,
and then even if don't need the row we still compute `total_size_sum`. Not sure
why it was done this way but this was my concern. Hope it answers your question.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
