[DB] Support SQL Field Query in Alcor #703

pkommoju · 2021-11-19T23:20:33Z

Ignite SqlFieldQuery changes.

Ignite allows fast searching of the KV store using a field (secondary key/index) in the value field through SQL interface.
This change is almost entirely in the commonlib (Ignite interface layer).
To enable this feature, add the annotation @QuerySqlField(index = true) to the field to index on.
When a class member is annotated with @QuerySqlField(index = true), a get() using filterParams will use SQLFieldsQuery if all the filter params are also in the set of QuerySqlFields.
If fields m1, m2, and m3 are in the QuerySqlField set, in that order, and thet get supplies only m1, or m1 and m2, or all three, then Ignite interface will use SqlFieldsQuery, otherwise it will fallback to Scan Query. This is called index prefix search.
One optimization not in this commit is to cache the queries and reuse them. If needed, this optimization can be added later.
For an example use, look at web...NodeInfo class. and services/sqlquery and services/scanquery.

Add document about transactional semantics for Alcor Caches in general and spefically about Ignite Caches.

cj-chung · 2021-11-25T00:42:05Z

lib/src/main/java/com/futurewei/alcor/common/entity/CustomerResource.java


 import java.util.Objects;

 public class CustomerResource extends Resource {

    @JsonProperty("project_id")
+    // @QuerySqlField(index = true)


@pkommoju If we don't need add @QuerySqlField here, please remove it and the import.

@DavidLiu506 Please confirm, if we need the index on project_id?

I will remove it after Dahai confirms that it is no longer needed.

Remove it first. Add it back if there are scenarios.

Removed the commented annotation.

Let's just leave comment here. If we need it in the future, we can enable it then.

...ger/src/main/java/com/futurewei/alcor/netwconfigmanager/NetworkConfigManagerApplication.java

cj-chung · 2021-11-25T00:51:57Z

web/src/main/java/com/futurewei/alcor/web/entity/node/NodeInfo.java

    private String id;

    @JsonProperty("node_name")
+    // @QuerySqlField(index = true)


@pkommoju and @DavidLiu506 Do we need index for node_id and node_name? If not, please remove it.

node_name needs index. In fact NMM swagger and the getAllNodes allows querying on any combination of name, mac, local ip, ncm_id. I should add index on all of them. id may not be needed if getAllNodes doesn't use SCAN query if querying by only id. I will check this path and remove index on id if this is so.

Removed index on id, added it on localIp, macAddress, ncm_id.

cj-chung · 2021-11-25T01:07:06Z

lib/src/main/java/com/futurewei/alcor/common/db/ignite/IgniteClientDbCache.java

+    }
+
+    @DurationStatistics
+    public <E1, E2> Map<K, V> getBySqlFieldsAll(Map<String, Object[]> filterParams) {


@pkommoju Do we need to throw exception from this function?

I think there is no need. runSqlFieldsQuery will throw any exception anyway. Just need to add throws CacheException.

cj-chung · 2021-11-25T01:14:45Z

lib/src/main/java/com/futurewei/alcor/common/db/ignite/IgniteClientDbCache.java

+            if (sqlFields != null && sqlFields.size() != 0) {
+                this.cache = getOrCreateIndexedCache(igniteClient, className, clientCacheConfig, null);
+                if (this.cache == null) {
+                    logger.log(Level.WARNING, "Create cache for client " + className + " with index failed, falling back");


@pkommoju How to handle falling back if this.cache=null ? just use igniteClient.getOrCreateCache(clientCacheConfig) instead here?

I am logging it as a warning message here. Few lines below, if this.cache is null, we call igniteClient.getOrCreateCache(clientCacheConfig). This call could also be because the the cache is not setup for SQLFieldsQuery in addition to handling the case where it is setup but something went wrong. Didn't want to have the call in two places.

xieus · 2021-11-29T22:14:42Z

@pkommoju There is a conflict with existing master after PR #704 was merged.

* Set index value inline size to length of GUUID to avoid multiple index page fetch penalty. * Remove unecessary logging. * Check only once to see if SQL index can be created.

lgtm-com · 2021-11-30T06:50:25Z

This pull request introduces 2 alerts when merging 5a1ea86 into a843622 - view on LGTM.com

new alerts:

2 for Dereferenced variable may be null

…facts

lgtm-com · 2021-11-30T20:31:50Z

This pull request introduces 2 alerts when merging af58b49 into a843622 - view on LGTM.com

new alerts:

2 for Dereferenced variable may be null

cj-chung · 2021-11-29T19:59:46Z

lib/src/main/java/com/futurewei/alcor/common/entity/CustomerResource.java


 import java.util.Objects;

 public class CustomerResource extends Resource {

    @JsonProperty("project_id")
+    // @QuerySqlField(index = true)


Let's just leave comment here. If we need it in the future, we can enable it then.

...ger/src/main/java/com/futurewei/alcor/netwconfigmanager/NetworkConfigManagerApplication.java

docs/modules/ROOT/pages/performance/sql_and_scan_query_pref_comp.adoc

lgtm-com · 2021-11-30T23:38:50Z

This pull request introduces 2 alerts when merging 02f04a5 into a843622 - view on LGTM.com

new alerts:

2 for Dereferenced variable may be null

cj-chung

@pkommoju Another comment: please rename the scanquery service to a name like scanquery_test_nodemanager.

In that case, should the sqlquery service also be renamed as sqlquery_test_nodemanager, to be consistant?

DavidLiu506 · 2021-11-24T22:06:41Z

lib/src/main/java/com/futurewei/alcor/common/db/ignite/IgniteClientDbCache.java

+                this.cache = getOrCreateIndexedCache(igniteClient, className, clientCacheConfig, null);
+                if (this.cache == null) {
+                    logger.log(Level.WARNING, "Create cache for client " + className + " with index failed, falling back");
+                }


Add if (!checkedForSqlFields)

This check is a few lines above.

DavidLiu506 · 2021-11-24T22:50:11Z

lib/src/main/java/com/futurewei/alcor/common/db/ignite/IgniteClientDbCache.java

+    }
+    private Map<String, SqlField> sqlFields = null; // needed for index creation and querying
+    private List<String> sqlColumns = null;         // needed for constructing indexes in specified order
+                                                    // so that a prefix query can use indexed search.


Remove sqlColumns , sqlFields use LinkedHashMap

Is it fixed?

Removed sqlColumns and changed Map to LinkedHashMap.

DavidLiu506 · 2021-11-25T02:01:44Z

lib/src/main/java/com/futurewei/alcor/common/entity/CustomerResource.java


 import java.util.Objects;

 public class CustomerResource extends Resource {

    @JsonProperty("project_id")
+    // @QuerySqlField(index = true)


Remove it first. Add it back if there are scenarios.

Removed the commented annotation.

DavidLiu506 · 2021-11-30T22:03:59Z

web/src/main/java/com/futurewei/alcor/web/entity/port/PortEntity.java

@@ -20,13 +20,15 @@
 import com.futurewei.alcor.common.entity.CustomerResource;
 import lombok.Data;
 import lombok.NoArgsConstructor;
+import org.apache.ignite.cache.query.annotations.QuerySqlField;

 import java.util.List;

 @Data
 @JsonInclude(JsonInclude.Include.NON_ABSENT)
 public class PortEntity extends CustomerResource {
    @JsonProperty("network_id")


Don't add index

…y as suggested in the code review

lgtm-com · 2021-12-02T00:35:20Z

This pull request introduces 2 alerts when merging eb19113 into a843622 - view on LGTM.com

new alerts:

2 for Dereferenced variable may be null

lgtm-com · 2021-12-02T01:08:59Z

This pull request introduces 2 alerts when merging 3cf7110 into a843622 - view on LGTM.com

new alerts:

2 for Dereferenced variable may be null

cj-chung

LGTM

pkommoju and others added 30 commits May 12, 2021 11:34

Documentation update

9645772

Add document about transactional semantics for Alcor Caches in general and spefically about Ignite Caches.

Apply changes suggested in review

183c36a

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

4896515

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

f170e1e

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

bcbf7b1

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

efc5b6e

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

c785697

Merge branch 'master' of https://github.com/pkommoju/alcor

d213c03

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

0f56454

Merge branch 'master' of https://github.com/pkommoju/alcor

4a2a382

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

7f93fe3

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

8d6dd0e

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

b19d428

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

539d0c0

Merge branch 'master' of https://github.com/pkommoju/alcor

6a9a1c6

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

19430b2

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

1d7ef7e

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

cd1fec6

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

e57abe4

Add missing file

1b63054

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

fe81008

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

118dd53

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

7d9a2e5

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

3884a44

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

0b5cd11

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

3f094be

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

bb777ec

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

c0e8c2d

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

206abc7

SQLFeildQuery changes (work in progress)

93e468c

cj-chung assigned pkommoju Nov 25, 2021

cj-chung reviewed Nov 25, 2021

View reviewed changes

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

6a95d98

xieus changed the title ~~Sqlfieldquery~~ [DB] Support SQL Field Query in Alcor Nov 29, 2021

Merge branch 'master' of https://github.com/futurewei-cloud/alcor

a8fce0e

xieus added this to the Version 1.0.2021.12.30 milestone Nov 29, 2021

pkommoju added 2 commits November 29, 2021 15:49

Fix merge conflict

272874b

* Address code review comments.

5a1ea86

* Set index value inline size to length of GUUID to avoid multiple index page fetch penalty. * Remove unecessary logging. * Check only once to see if SQL index can be created.

Add SQL and Scan query performance comparison report and related arti…

af58b49

…facts

cj-chung reviewed Nov 30, 2021

View reviewed changes

Address more code review comments

02f04a5

cj-chung suggested changes Dec 1, 2021

View reviewed changes

DavidLiu506 reviewed Dec 1, 2021

View reviewed changes

pkommoju added 2 commits December 1, 2021 14:19

Add insert times, remove sqlColumns and rename sqlquery and scan quer…

913cf6c

…y as suggested in the code review

Fix compile error and IntelliJ issues

eb19113

Set logging level to default

3cf7110

DavidLiu506 approved these changes Dec 2, 2021

View reviewed changes

cj-chung approved these changes Dec 2, 2021

View reviewed changes

xieus merged commit 03bed53 into futurewei-cloud:master Dec 2, 2021

xieus mentioned this pull request Dec 2, 2021

[Netw Config Manager] Support coexist of gRPC and REST APIs #706

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DB] Support SQL Field Query in Alcor #703

[DB] Support SQL Field Query in Alcor #703

pkommoju commented Nov 19, 2021

cj-chung Nov 25, 2021

cj-chung Nov 25, 2021

pkommoju Nov 25, 2021

DavidLiu506 Nov 25, 2021 •

edited by pkommoju

Loading

cj-chung Nov 29, 2021

pkommoju Nov 30, 2021

cj-chung Nov 25, 2021

pkommoju Nov 25, 2021

pkommoju Nov 29, 2021

cj-chung Nov 25, 2021

pkommoju Nov 25, 2021

cj-chung Nov 25, 2021

pkommoju Nov 25, 2021

xieus commented Nov 29, 2021

lgtm-com bot commented Nov 30, 2021

lgtm-com bot commented Nov 30, 2021

cj-chung Nov 29, 2021

lgtm-com bot commented Nov 30, 2021

cj-chung left a comment •

edited by pkommoju

Loading

DavidLiu506 Nov 24, 2021

pkommoju Dec 1, 2021

DavidLiu506 Nov 24, 2021

DavidLiu506 Nov 30, 2021

pkommoju Dec 1, 2021

DavidLiu506 Nov 25, 2021 •

edited by pkommoju

Loading

DavidLiu506 Nov 30, 2021

lgtm-com bot commented Dec 2, 2021

lgtm-com bot commented Dec 2, 2021

cj-chung left a comment

[DB] Support SQL Field Query in Alcor #703

[DB] Support SQL Field Query in Alcor #703

Conversation

pkommoju commented Nov 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DavidLiu506 Nov 25, 2021 • edited by pkommoju Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xieus commented Nov 29, 2021

lgtm-com bot commented Nov 30, 2021

lgtm-com bot commented Nov 30, 2021

Choose a reason for hiding this comment

lgtm-com bot commented Nov 30, 2021

cj-chung left a comment • edited by pkommoju Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DavidLiu506 Nov 25, 2021 • edited by pkommoju Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgtm-com bot commented Dec 2, 2021

lgtm-com bot commented Dec 2, 2021

cj-chung left a comment

Choose a reason for hiding this comment

DavidLiu506 Nov 25, 2021 •

edited by pkommoju

Loading

cj-chung left a comment •

edited by pkommoju

Loading

DavidLiu506 Nov 25, 2021 •

edited by pkommoju

Loading