[
https://issues.apache.org/jira/browse/IGNITE-6702?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16265026#comment-16265026
]
Vladimir Ozerov edited comment on IGNITE-6702 at 11/24/17 8:12 AM:
-------------------------------------------------------------------
[~kirill.shirokov], my comments:
1) I did a number of changes in the code to better align it with our styling
rules and general guidelines:
1.1) Variables should be abbreviated according to our abbreviation rules
1.2) It is better to avoid long chains of method calls on a single line because
it makes debug and log analysis harder.
1.3) Semantically different lines should have blank line in betwee,
2) {{H2TreeIndex#filter}} - there was potential NPE because thread-local
context could be {{null}}. To avoid that I moved cache filter logic to a common
method used in several places.
3) {{H2TreeIndex#filter}} - TODOs are not allowed in the code.
4) {{H2TreeIndex#filter}} - please avoid anonymous classes because according to
our experience it may lead to unexpected issues with binary compatibility. It
is better to rework it to top-level class or at least inner static class.
5) The most important - please confirm that we have enough tests for
{{COUNT(*)}} with different cache modes ({{PARTITIONED}}, {{PARTITIONED}} with
backups, {{REPLICATED}}, {{LOCAL}}, one nodes, several nodes). If no, let's
create a set of tests covering all these scenarios.
The rest looks good to me.
was (Author: vozerov):
[~kirill.shirokov], my comments:
1) I did a number of changes in the code to better align it with our styling
rules and general guidelines:
1.1) Variables should be abbreviated according to our abbreviation rules
1.2) It is better to avoid long chains of method calls on a single line because
it makes debug and log analysis harder.
1.3) Semantically different lines should have blank line in betwee,
2) {{H2TreeIndex#filter}} - there was potential NPE because thread-locl context
could be {{null}}. To avoid that I moved cache filter logic to a common method
use in several places.
3) {{H2TreeIndex#filter}} - TODOs are not allowed in the code.
4) {{H2TreeIndex#filter}} - please avoid anonymous classes because according to
our experience it may lead to unexpected issues with binary compatibility. It
is better to rework it to top-level class or at least inner static class.
5) The most important - please confirm that we have enough tests for
{{COUNT(*)}} with different cache modes ({{PARTITIONED}}, {{PARTITIONED}} with
backups, {{REPLICATED}}, {{LOCAL}}, one nodes, several nodes). If no, let's
create a set of tests covering all these scenarios.
> Get rid of values deserialization in H2TreeIndex.getRowCount
> ------------------------------------------------------------
>
> Key: IGNITE-6702
> URL: https://issues.apache.org/jira/browse/IGNITE-6702
> Project: Ignite
> Issue Type: Improvement
> Components: sql
> Reporter: Semen Boikov
> Assignee: Kirill Shirokov
> Labels: performance
> Fix For: 2.4
>
>
> It seems H2TreeIndex.getRowCount is called for 'select count(*) from x'
> queries, now it is implemented via BPlusTree.find(x, x) method which
> deserializes all values. Actually values are not needed there, need implement
> method on BPlusTree computing size without deserialization.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
