[jira] [Commented] (HBASE-20711) Save on a Cell iteration when writing
[
https://issues.apache.org/jira/browse/HBASE-20711?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17021069#comment-17021069
]
HBase QA commented on HBASE-20711:
--
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 1m
38s{color} | {color:blue} Docker mode activated. {color} |
| {color:red}-1{color} | {color:red} yetus {color} | {color:red} 0m 11s{color}
| {color:red} Unprocessed flag(s): --skip-errorprone --quick-hadoopcheck
{color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=19.03.4 Server=19.03.4 base:
https://builds.apache.org/job/PreCommit-HBASE-Build/1093/artifact/patchprocess/Dockerfile
|
| JIRA Issue | HBASE-20711 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12927175/HBASE-20711.branch-2.0.001.patch
|
| Console output |
https://builds.apache.org/job/PreCommit-HBASE-Build/1093/console |
| versions | git=2.11.0 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
> Save on a Cell iteration when writing
> -
>
> Key: HBASE-20711
> URL: https://issues.apache.org/jira/browse/HBASE-20711
> Project: HBase
> Issue Type: Bug
> Components: Performance
>Reporter: Michael Stack
>Assignee: Michael Stack
>Priority: Minor
> Attachments: HBASE-20711.branch-2.0.001.patch
>
>
> This is a minor savings. We were doing a spin through all Cells on receipt
> just to check their size when subsequently, we were doing an iteration of all
> Cells to insert. It manifest as a little spike in perf output. This change
> removes the purposed spin through Cells and just does size check as part of
> the general Cell insert (perf spike no longer shows but the cost of the size
> check still remains).
> There is also a minor bug fix where on receipt we were using the Puts row
> rather than the Cells row; client may have succeeded in submitting a Cell
> that disagreed with the hosting Mutation and it would have been written as
> something else altogether -- with the Puts row -- rather than being rejected.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (HBASE-20711) Save on a Cell iteration when writing
[ https://issues.apache.org/jira/browse/HBASE-20711?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17020751#comment-17020751 ] Michael Stack commented on HBASE-20711: --- Move old issue that still seems legit but that is having no work being done out of its parent issue. > Save on a Cell iteration when writing > - > > Key: HBASE-20711 > URL: https://issues.apache.org/jira/browse/HBASE-20711 > Project: HBase > Issue Type: Bug > Components: Performance >Reporter: Michael Stack >Assignee: Michael Stack >Priority: Minor > Attachments: HBASE-20711.branch-2.0.001.patch > > > This is a minor savings. We were doing a spin through all Cells on receipt > just to check their size when subsequently, we were doing an iteration of all > Cells to insert. It manifest as a little spike in perf output. This change > removes the purposed spin through Cells and just does size check as part of > the general Cell insert (perf spike no longer shows but the cost of the size > check still remains). > There is also a minor bug fix where on receipt we were using the Puts row > rather than the Cells row; client may have succeeded in submitting a Cell > that disagreed with the hosting Mutation and it would have been written as > something else altogether -- with the Puts row -- rather than being rejected. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (HBASE-20711) Save on a Cell iteration when writing
[ https://issues.apache.org/jira/browse/HBASE-20711?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16551729#comment-16551729 ] stack commented on HBASE-20711: --- ... just to say that I was looking or a 'cheap' win -- thats me -- but [~chia7712] 's review (thankfully) points out that more work is needed. > Save on a Cell iteration when writing > - > > Key: HBASE-20711 > URL: https://issues.apache.org/jira/browse/HBASE-20711 > Project: HBase > Issue Type: Sub-task > Components: Performance >Reporter: stack >Assignee: stack >Priority: Minor > Attachments: HBASE-20711.branch-2.0.001.patch > > > This is a minor savings. We were doing a spin through all Cells on receipt > just to check their size when subsequently, we were doing an iteration of all > Cells to insert. It manifest as a little spike in perf output. This change > removes the purposed spin through Cells and just does size check as part of > the general Cell insert (perf spike no longer shows but the cost of the size > check still remains). > There is also a minor bug fix where on receipt we were using the Puts row > rather than the Cells row; client may have succeeded in submitting a Cell > that disagreed with the hosting Mutation and it would have been written as > something else altogether -- with the Puts row -- rather than being rejected. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-20711) Save on a Cell iteration when writing
[ https://issues.apache.org/jira/browse/HBASE-20711?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16551315#comment-16551315 ] stack commented on HBASE-20711: --- That would be sweet [~mdrob]. Thanks. > Save on a Cell iteration when writing > - > > Key: HBASE-20711 > URL: https://issues.apache.org/jira/browse/HBASE-20711 > Project: HBase > Issue Type: Sub-task > Components: Performance >Reporter: stack >Assignee: stack >Priority: Minor > Attachments: HBASE-20711.branch-2.0.001.patch > > > This is a minor savings. We were doing a spin through all Cells on receipt > just to check their size when subsequently, we were doing an iteration of all > Cells to insert. It manifest as a little spike in perf output. This change > removes the purposed spin through Cells and just does size check as part of > the general Cell insert (perf spike no longer shows but the cost of the size > check still remains). > There is also a minor bug fix where on receipt we were using the Puts row > rather than the Cells row; client may have succeeded in submitting a Cell > that disagreed with the hosting Mutation and it would have been written as > something else altogether -- with the Puts row -- rather than being rejected. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-20711) Save on a Cell iteration when writing
[ https://issues.apache.org/jira/browse/HBASE-20711?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16551234#comment-16551234 ] Mike Drob commented on HBASE-20711: --- I may have time to pick it up next week. > Save on a Cell iteration when writing > - > > Key: HBASE-20711 > URL: https://issues.apache.org/jira/browse/HBASE-20711 > Project: HBase > Issue Type: Sub-task > Components: Performance >Reporter: stack >Assignee: stack >Priority: Minor > Attachments: HBASE-20711.branch-2.0.001.patch > > > This is a minor savings. We were doing a spin through all Cells on receipt > just to check their size when subsequently, we were doing an iteration of all > Cells to insert. It manifest as a little spike in perf output. This change > removes the purposed spin through Cells and just does size check as part of > the general Cell insert (perf spike no longer shows but the cost of the size > check still remains). > There is also a minor bug fix where on receipt we were using the Puts row > rather than the Cells row; client may have succeeded in submitting a Cell > that disagreed with the hosting Mutation and it would have been written as > something else altogether -- with the Puts row -- rather than being rejected. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-20711) Save on a Cell iteration when writing
[ https://issues.apache.org/jira/browse/HBASE-20711?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16551220#comment-16551220 ] stack commented on HBASE-20711: --- No. I should be though. You want to take it [~mdrob]? > Save on a Cell iteration when writing > - > > Key: HBASE-20711 > URL: https://issues.apache.org/jira/browse/HBASE-20711 > Project: HBase > Issue Type: Sub-task > Components: Performance >Reporter: stack >Assignee: stack >Priority: Minor > Attachments: HBASE-20711.branch-2.0.001.patch > > > This is a minor savings. We were doing a spin through all Cells on receipt > just to check their size when subsequently, we were doing an iteration of all > Cells to insert. It manifest as a little spike in perf output. This change > removes the purposed spin through Cells and just does size check as part of > the general Cell insert (perf spike no longer shows but the cost of the size > check still remains). > There is also a minor bug fix where on receipt we were using the Puts row > rather than the Cells row; client may have succeeded in submitting a Cell > that disagreed with the hosting Mutation and it would have been written as > something else altogether -- with the Puts row -- rather than being rejected. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-20711) Save on a Cell iteration when writing
[ https://issues.apache.org/jira/browse/HBASE-20711?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16551190#comment-16551190 ] Mike Drob commented on HBASE-20711: --- [~stack] - are you still working on this? > Save on a Cell iteration when writing > - > > Key: HBASE-20711 > URL: https://issues.apache.org/jira/browse/HBASE-20711 > Project: HBase > Issue Type: Sub-task > Components: Performance >Reporter: stack >Assignee: stack >Priority: Minor > Attachments: HBASE-20711.branch-2.0.001.patch > > > This is a minor savings. We were doing a spin through all Cells on receipt > just to check their size when subsequently, we were doing an iteration of all > Cells to insert. It manifest as a little spike in perf output. This change > removes the purposed spin through Cells and just does size check as part of > the general Cell insert (perf spike no longer shows but the cost of the size > check still remains). > There is also a minor bug fix where on receipt we were using the Puts row > rather than the Cells row; client may have succeeded in submitting a Cell > that disagreed with the hosting Mutation and it would have been written as > something else altogether -- with the Puts row -- rather than being rejected. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-20711) Save on a Cell iteration when writing
[
https://issues.apache.org/jira/browse/HBASE-20711?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16521370#comment-16521370
]
stack commented on HBASE-20711:
---
bq. Do you mean the path using the CellScanner (see below)?
No. I meant this:
{code}
BTW, why doing the following change? The change makes us copy the row many
times. Is it related another bugfix boss?
- .setRow(put.getRow())
+ cell = cellBuilder.clear()
+ .setRow(proto.getRow().toByteArray())
{code}
I was afraid that the row in the Put might not match what was in the Cell. If
it didn't, somehow, then we'd pass in a Cell that was not supplied by the
client?
I like your point that it means we'll make a new row each time... that we
should avoid that if we can.
I see this size check costing us about 3% of CPU when writing. Maybe 1% is the
iteration cost? Less? If it makes the code easier, we should keep it as is?
> Save on a Cell iteration when writing
> -
>
> Key: HBASE-20711
> URL: https://issues.apache.org/jira/browse/HBASE-20711
> Project: HBase
> Issue Type: Sub-task
> Components: Performance
>Reporter: stack
>Assignee: stack
>Priority: Minor
> Attachments: HBASE-20711.branch-2.0.001.patch
>
>
> This is a minor savings. We were doing a spin through all Cells on receipt
> just to check their size when subsequently, we were doing an iteration of all
> Cells to insert. It manifest as a little spike in perf output. This change
> removes the purposed spin through Cells and just does size check as part of
> the general Cell insert (perf spike no longer shows but the cost of the size
> check still remains).
> There is also a minor bug fix where on receipt we were using the Puts row
> rather than the Cells row; client may have succeeded in submitting a Cell
> that disagreed with the hosting Mutation and it would have been written as
> something else altogether -- with the Puts row -- rather than being rejected.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-20711) Save on a Cell iteration when writing
[
https://issues.apache.org/jira/browse/HBASE-20711?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16507758#comment-16507758
]
Chia-Ping Tsai commented on HBASE-20711:
{code:java}
+ public static Put toPut(final MutationProto proto, final CellScanner
cellScanner)
+ throws IOException {
+return toPut(proto, cellScanner, 0);
+ }{code}
Should we pass the Long.MAX_VALUE to replace the zero? Seems the zero makes all
cells exceed the limit?
> Save on a Cell iteration when writing
> -
>
> Key: HBASE-20711
> URL: https://issues.apache.org/jira/browse/HBASE-20711
> Project: HBase
> Issue Type: Sub-task
> Components: Performance
>Reporter: stack
>Assignee: stack
>Priority: Minor
> Attachments: HBASE-20711.branch-2.0.001.patch
>
>
> This is a minor savings. We were doing a spin through all Cells on receipt
> just to check their size when subsequently, we were doing an iteration of all
> Cells to insert. It manifest as a little spike in perf output. This change
> removes the purposed spin through Cells and just does size check as part of
> the general Cell insert (perf spike no longer shows but the cost of the size
> check still remains).
> There is also a minor bug fix where on receipt we were using the Puts row
> rather than the Cells row; client may have succeeded in submitting a Cell
> that disagreed with the hosting Mutation and it would have been written as
> something else altogether -- with the Puts row -- rather than being rejected.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-20711) Save on a Cell iteration when writing
[
https://issues.apache.org/jira/browse/HBASE-20711?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16507755#comment-16507755
]
Chia-Ping Tsai commented on HBASE-20711:
{quote}There is also a minor bug fix where on receipt we were using the Puts
row rather than the Cells row; client may have succeeded in submitting a Cell
that disagreed with the hosting Mutation and it would have been written as
something else altogether – with the Puts row – rather than being rejected.
On the bugfix, makes sense to you sir?
{quote}
Do you mean the path using the CellScanner (see below)? If so, I prefer to
reject the invalid cells (when calling the Mutation#add(Cell), the invalid cell
will be rejected by throwing the WrongRowIOException. Of course, we have to
skip the remaining cells in this case...Another issue :() rather than
swallowing them since it means something unexpected behaviors happen on our
hbase user's code. Correcting user's data without any alert may delay user to
notice the truly issue.
{code:java}
if (put == null) {
put = new Put(cell.getRowArray(), cell.getRowOffset(),
cell.getRowLength(), timestamp);
}
+checkCellSizeLimit(maxCellSize, cell);
put.add(cell);
}{code}
BTW, why doing the following change? The change makes us copy the row many
times. Is it related another bugfix boss?
{code:java}
- .setRow(put.getRow())
+ cell = cellBuilder.clear()
+ .setRow(proto.getRow().toByteArray()){code}
> Save on a Cell iteration when writing
> -
>
> Key: HBASE-20711
> URL: https://issues.apache.org/jira/browse/HBASE-20711
> Project: HBase
> Issue Type: Sub-task
> Components: Performance
>Reporter: stack
>Assignee: stack
>Priority: Minor
> Attachments: HBASE-20711.branch-2.0.001.patch
>
>
> This is a minor savings. We were doing a spin through all Cells on receipt
> just to check their size when subsequently, we were doing an iteration of all
> Cells to insert. It manifest as a little spike in perf output. This change
> removes the purposed spin through Cells and just does size check as part of
> the general Cell insert (perf spike no longer shows but the cost of the size
> check still remains).
> There is also a minor bug fix where on receipt we were using the Puts row
> rather than the Cells row; client may have succeeded in submitting a Cell
> that disagreed with the hosting Mutation and it would have been written as
> something else altogether -- with the Puts row -- rather than being rejected.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-20711) Save on a Cell iteration when writing
[ https://issues.apache.org/jira/browse/HBASE-20711?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16507648#comment-16507648 ] stack commented on HBASE-20711: --- Good point [~chia7712] On the bugfix, makes sense to you sir? > Save on a Cell iteration when writing > - > > Key: HBASE-20711 > URL: https://issues.apache.org/jira/browse/HBASE-20711 > Project: HBase > Issue Type: Sub-task > Components: Performance >Reporter: stack >Assignee: stack >Priority: Minor > Attachments: HBASE-20711.branch-2.0.001.patch > > > This is a minor savings. We were doing a spin through all Cells on receipt > just to check their size when subsequently, we were doing an iteration of all > Cells to insert. It manifest as a little spike in perf output. This change > removes the purposed spin through Cells and just does size check as part of > the general Cell insert (perf spike no longer shows but the cost of the size > check still remains). > There is also a minor bug fix where on receipt we were using the Puts row > rather than the Cells row; client may have succeeded in submitting a Cell > that disagreed with the hosting Mutation and it would have been written as > something else altogether -- with the Puts row -- rather than being rejected. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-20711) Save on a Cell iteration when writing
[
https://issues.apache.org/jira/browse/HBASE-20711?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16507562#comment-16507562
]
Chia-Ping Tsai commented on HBASE-20711:
If we do the size check in converting the proto to mutation, we have to record
the index of CellScanner in order to ignore the correct number of cells when
encountering the exception.
{code:java}
int processedMutationIndex = 0;
for (Action mutation : mutations) {
// The non-null mArray[i] means the cell scanner has been read.
if (mArray[processedMutationIndex++] == null) {
skipCellsForMutation(mutation, cells);
}
}{code}
> Save on a Cell iteration when writing
> -
>
> Key: HBASE-20711
> URL: https://issues.apache.org/jira/browse/HBASE-20711
> Project: HBase
> Issue Type: Sub-task
> Components: Performance
>Reporter: stack
>Assignee: stack
>Priority: Minor
> Attachments: HBASE-20711.branch-2.0.001.patch
>
>
> This is a minor savings. We were doing a spin through all Cells on receipt
> just to check their size when subsequently, we were doing an iteration of all
> Cells to insert. It manifest as a little spike in perf output. This change
> removes the purposed spin through Cells and just does size check as part of
> the general Cell insert (perf spike no longer shows but the cost of the size
> check still remains).
> There is also a minor bug fix where on receipt we were using the Puts row
> rather than the Cells row; client may have succeeded in submitting a Cell
> that disagreed with the hosting Mutation and it would have been written as
> something else altogether -- with the Puts row -- rather than being rejected.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-20711) Save on a Cell iteration when writing
[
https://issues.apache.org/jira/browse/HBASE-20711?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16507211#comment-16507211
]
Hadoop QA commented on HBASE-20711:
---
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
19s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green} 0m
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
|| || || || {color:brown} branch-2.0 Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 1m
4s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 5m
14s{color} | {color:green} branch-2.0 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m
9s{color} | {color:green} branch-2.0 passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m
31s{color} | {color:green} branch-2.0 passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green} 3m
40s{color} | {color:green} branch has no errors when building our shaded
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 2m
37s{color} | {color:green} branch-2.0 passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
51s{color} | {color:green} branch-2.0 passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
15s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 3m
11s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m
1s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 2m
1s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 0m
30s{color} | {color:red} hbase-client: The patch generated 1 new + 182
unchanged - 0 fixed = 183 total (was 182) {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green} 3m
38s{color} | {color:green} patch has no errors when building our shaded
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green}
10m 8s{color} | {color:green} Patch does not cause any errors with Hadoop
2.6.5 2.7.4 or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 2m
49s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
44s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 1m 46s{color}
| {color:red} hbase-client in the patch failed. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red}143m 40s{color}
| {color:red} hbase-server in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
40s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}188m 28s{color} |
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests | hadoop.hbase.shaded.protobuf.TestProtobufUtil |
| | hadoop.hbase.client.TestFromClientSide |
| | hadoop.hbase.client.TestFromClientSideWithCoprocessor |
| | hadoop.hbase.client.TestMalformedCellFromClient |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:6f01af0 |
| JIRA Issue | HBASE-20711 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12927175/HBASE-20711.branch-2.0.001.patch
|
| Optional Tests | asflicense javac javadoc unit findbugs shadedjars
hadoopcheck
