You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/12/20 16:02:03 UTC
[GitHub] [ignite] J-Bakuli opened a new pull request, #10453: IGNITE-17321 Document which api can work with partition awareness
J-Bakuli opened a new pull request, #10453:
URL: https://github.com/apache/ignite/pull/10453
In javadoc org.apache.ignite.configuration.ClientConfiguration#partitionAwarenessEnabled and in the description of functionality
https://ignite.apache.org/docs/latest/thin-clients/java-thin-client#partition-awareness, it is not described with which api this functionality will work and in what cases. For example, will it work with getAll, in a transaction?
Describe in the documentation and in the javadoc in which cases it works and with which api.
______________
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
### The Contribution Checklist
- [ ] There is a single JIRA ticket related to the pull request.
- [ ] The web-link to the pull request is attached to the JIRA ticket.
- [ ] The JIRA ticket has the _Patch Available_ state.
- [ ] The pull request body describes changes that have been made.
The description explains _WHAT_ and _WHY_ was made instead of _HOW_.
- [ ] The pull request title is treated as the final commit message.
The following pattern must be used: `IGNITE-XXXX Change summary` where `XXXX` - number of JIRA issue.
- [ ] A reviewer has been mentioned through the JIRA comments
(see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers))
- [ ] The pull request has been checked by the Teamcity Bot and
the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
### Notes
- [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
- [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
- [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
- [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] J-Bakuli commented on a diff in pull request #10453: IGNITE-17321 Document which api can work with partition awareness
Posted by "J-Bakuli (via GitHub)" <gi...@apache.org>.
J-Bakuli commented on code in PR #10453:
URL: https://github.com/apache/ignite/pull/10453#discussion_r1118801674
##########
docs/_docs/thin-clients/java-thin-client.adoc:
##########
@@ -79,6 +79,10 @@ Note that the code above provides a failover mechanism in case of server node fa
include::includes/partition-awareness.adoc[]
+Partition awareness functionality helps to avoid an additional network hop in the following scenarios:
+1. Single-key operations API, like #put, #get, etc. However, the functionality has no effect on those operations within explicit transactions (initiated via ClientTransaction.txStart() described in <<Executed transactions>> section).
Review Comment:
I've corrected the links to Transactions and IndexQueries
Thank you
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a diff in pull request #10453: IGNITE-17321 Document which api can work with partition awareness
Posted by "timoninmaxim (via GitHub)" <gi...@apache.org>.
timoninmaxim commented on code in PR #10453:
URL: https://github.com/apache/ignite/pull/10453#discussion_r1131607202
##########
modules/core/src/main/java/org/apache/ignite/configuration/ClientConfiguration.java:
##########
@@ -529,11 +531,17 @@ public ClientConfiguration setTransactionConfiguration(ClientTransactionConfigur
}
/**
+ * Partition awareness functionality helps to avoid an additional network hop in the following scenarios:
+ * <ul>
+ * <li>1. Single-key operations API, like put(), get(), etc. However, the functionality has no effect on those
+ * operations within explicit transactions {@code ClientTransactions#txStart()}.</li>
+ * <li>2. {@link ScanQuery#setPartition(Integer)} and {@link IndexQuery#setPartition(Integer)} accept a
+ * partition number as a parameter with which the query is routed to a particular server node that stores
+ * the requested data.</li>
+ * </ul>
* @return A value indicating whether partition awareness should be enabled.
- * <p>
- * Default is {@code true}: client sends requests directly to the primary node for the given cache key.
- * To do so, connection is established to every known server node.
- * <p>
+ * <p>Default is {@code true}: client sends requests directly to the primary node for the given cache key.
Review Comment:
Let's move this part at the beginning of javadoc. @return should be the last line in the doc:
```
<p>Default is ....
When {@code false}...
<p>
Partition awareness functionality...
```
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a diff in pull request #10453: IGNITE-17321 Document which api can work with partition awareness
Posted by "timoninmaxim (via GitHub)" <gi...@apache.org>.
timoninmaxim commented on code in PR #10453:
URL: https://github.com/apache/ignite/pull/10453#discussion_r1131604213
##########
modules/core/src/main/java/org/apache/ignite/configuration/ClientConfiguration.java:
##########
@@ -529,11 +531,17 @@ public ClientConfiguration setTransactionConfiguration(ClientTransactionConfigur
}
/**
+ * Partition awareness functionality helps to avoid an additional network hop in the following scenarios:
+ * <ul>
+ * <li>1. Single-key operations API, like put(), get(), etc. However, the functionality has no effect on those
+ * operations within explicit transactions {@code ClientTransactions#txStart()}.</li>
Review Comment:
here and below, let's replace @code with @link, and then import required dependencies, to make it possible to navigate for mentioned functions
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] J-Bakuli commented on a diff in pull request #10453: IGNITE-17321 Document which api can work with partition awareness
Posted by "J-Bakuli (via GitHub)" <gi...@apache.org>.
J-Bakuli commented on code in PR #10453:
URL: https://github.com/apache/ignite/pull/10453#discussion_r1132019810
##########
modules/core/src/main/java/org/apache/ignite/configuration/ClientConfiguration.java:
##########
@@ -529,11 +531,17 @@ public ClientConfiguration setTransactionConfiguration(ClientTransactionConfigur
}
/**
+ * Partition awareness functionality helps to avoid an additional network hop in the following scenarios:
+ * <ul>
+ * <li>1. Single-key operations API, like put(), get(), etc. However, the functionality has no effect on those
+ * operations within explicit transactions {@code ClientTransactions#txStart()}.</li>
+ * <li>2. {@link ScanQuery#setPartition(Integer)} and {@link IndexQuery#setPartition(Integer)} accept a
+ * partition number as a parameter with which the query is routed to a particular server node that stores
+ * the requested data.</li>
+ * </ul>
* @return A value indicating whether partition awareness should be enabled.
- * <p>
- * Default is {@code true}: client sends requests directly to the primary node for the given cache key.
- * To do so, connection is established to every known server node.
- * <p>
+ * <p>Default is {@code true}: client sends requests directly to the primary node for the given cache key.
Review Comment:
thank you, I will correct
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim merged pull request #10453: IGNITE-17321 Add docs for java client partition awareness use cases
Posted by "timoninmaxim (via GitHub)" <gi...@apache.org>.
timoninmaxim merged PR #10453:
URL: https://github.com/apache/ignite/pull/10453
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] J-Bakuli commented on a diff in pull request #10453: IGNITE-17321 Document which api can work with partition awareness
Posted by "J-Bakuli (via GitHub)" <gi...@apache.org>.
J-Bakuli commented on code in PR #10453:
URL: https://github.com/apache/ignite/pull/10453#discussion_r1126284822
##########
modules/core/src/main/java/org/apache/ignite/configuration/ClientConfiguration.java:
##########
@@ -529,6 +531,11 @@ public ClientConfiguration setTransactionConfiguration(ClientTransactionConfigur
}
/**
+ * Partition awareness functionality helps to avoid an additional network hop in the following scenarios:
+ * 1. Single-key operations API, like put(), get(), etc. However, the functionality has no effect on those operations
Review Comment:
thank you, I have corrected
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] J-Bakuli commented on a diff in pull request #10453: IGNITE-17321 Document which api can work with partition awareness
Posted by "J-Bakuli (via GitHub)" <gi...@apache.org>.
J-Bakuli commented on code in PR #10453:
URL: https://github.com/apache/ignite/pull/10453#discussion_r1132003013
##########
modules/core/src/main/java/org/apache/ignite/configuration/ClientConfiguration.java:
##########
@@ -529,11 +531,17 @@ public ClientConfiguration setTransactionConfiguration(ClientTransactionConfigur
}
/**
+ * Partition awareness functionality helps to avoid an additional network hop in the following scenarios:
+ * <ul>
+ * <li>1. Single-key operations API, like put(), get(), etc. However, the functionality has no effect on those
+ * operations within explicit transactions {@code ClientTransactions#txStart()}.</li>
Review Comment:
thank you, I will correct
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a diff in pull request #10453: IGNITE-17321 Document which api can work with partition awareness
Posted by "timoninmaxim (via GitHub)" <gi...@apache.org>.
timoninmaxim commented on code in PR #10453:
URL: https://github.com/apache/ignite/pull/10453#discussion_r1109446143
##########
docs/_docs/thin-clients/java-thin-client.adoc:
##########
@@ -79,6 +79,10 @@ Note that the code above provides a failover mechanism in case of server node fa
include::includes/partition-awareness.adoc[]
+Partition awareness functionality helps to avoid an additional network hop in the following scenarios:
+1. Single-key operations API, like #put, #get, etc. However, the functionality has no effect on those operations within explicit transactions (initiated via ClientTransaction.txStart() described in <<Executed transactions>> section).
Review Comment:
I set up docs locally and link to "Executing Scan Queries" works, but links on transactions and IndexQueries doesn't. Could you, please, check it?
##########
modules/core/src/main/java/org/apache/ignite/configuration/ClientConfiguration.java:
##########
@@ -525,6 +527,11 @@ public ClientConfiguration setTransactionConfiguration(ClientTransactionConfigur
}
/**
+ * Used with single-key operations API, and it does not work for those operations within explicit transactions
Review Comment:
You should also change docs here
##########
docs/_docs/thin-clients/java-thin-client.adoc:
##########
@@ -79,6 +79,10 @@ Note that the code above provides a failover mechanism in case of server node fa
include::includes/partition-awareness.adoc[]
+Partition awareness functionality helps to avoid an additional network hop in the following scenarios:
+1. Single-key operations API, like #put, #get, etc. However, the functionality has no effect on those operations within explicit transactions (initiated via ClientTransaction.txStart() described in <<Executed transactions>> section).
Review Comment:
"#put, #get" -> `put(), get()`
##########
docs/_docs/code-snippets/java/src/main/java/org/apache/ignite/snippets/JavaThinClient.java:
##########
@@ -359,6 +359,9 @@ void partitionAwareness() throws Exception {
try (IgniteClient client = Ignition.startClient(cfg)) {
ClientCache<Integer, String> cache = client.cache("myCache");
// Put, get or remove data from the cache...
+ cache.put(0; "Hello, world!");
Review Comment:
replace ';' with ','
##########
docs/_docs/thin-clients/java-thin-client.adoc:
##########
@@ -79,6 +79,10 @@ Note that the code above provides a failover mechanism in case of server node fa
include::includes/partition-awareness.adoc[]
+Partition awareness functionality helps to avoid an additional network hop in the following scenarios:
+1. Single-key operations API, like #put, #get, etc. However, the functionality has no effect on those operations within explicit transactions (initiated via ClientTransaction.txStart() described in <<Executed transactions>> section).
+2. ScanQuery and IndexQuery accept a partition number as a parameter with which the query is routed to a particular server node that stores the requested data. Refer to <<Executing Scan Queries>> and <<Executing Index Queries>> sections for more information.
Review Comment:
Also, you should mark numerations correctly - to enforce every point starts from new line. Currently, it is showed in a single line.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] J-Bakuli commented on a diff in pull request #10453: IGNITE-17321 Document which api can work with partition awareness
Posted by "J-Bakuli (via GitHub)" <gi...@apache.org>.
J-Bakuli commented on code in PR #10453:
URL: https://github.com/apache/ignite/pull/10453#discussion_r1126284500
##########
docs/_docs/thin-clients/java-thin-client.adoc:
##########
@@ -79,6 +79,12 @@ Note that the code above provides a failover mechanism in case of server node fa
include::includes/partition-awareness.adoc[]
+Partition awareness functionality helps to avoid an additional network hop in the following scenarios:
+
+1. Single-key operations API, like put(), get(), etc. However, the functionality has no effect on those operations within explicit transactions (initiated via ClientTransaction.txStart() described in <<Transactions>> section).
+
+2. ScanQuery and IndexQuery accept a partition number as a parameter with which the query is routed to a particular server node that stores the requested data. Refer to <<Executing Scan Queries>> and link:key-value-api/using-cache-queries[Executing Index Queries] sections for more information.
Review Comment:
thank you
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a diff in pull request #10453: IGNITE-17321 Document which api can work with partition awareness
Posted by "timoninmaxim (via GitHub)" <gi...@apache.org>.
timoninmaxim commented on code in PR #10453:
URL: https://github.com/apache/ignite/pull/10453#discussion_r1121345467
##########
modules/core/src/main/java/org/apache/ignite/configuration/ClientConfiguration.java:
##########
@@ -529,6 +531,11 @@ public ClientConfiguration setTransactionConfiguration(ClientTransactionConfigur
}
/**
+ * Partition awareness functionality helps to avoid an additional network hop in the following scenarios:
+ * 1. Single-key operations API, like put(), get(), etc. However, the functionality has no effect on those operations
Review Comment:
You should mark javadoc correctly. There are 2 examples:
1. Bad - check the javadocs for IndexQuery, it uses new line for splitting points and it doesn't work.
2. Good example - GridEncryptionManager, it uses tags "li", "ul", and it works.
The difference of you can check here - https://javadoc.io/doc/org.apache.ignite/ignite-core/2.14.0/index.html. Unfortunately it's impossible to provide direct links to the classes javadocs, but you can easily find them in a class list.
##########
docs/_docs/thin-clients/java-thin-client.adoc:
##########
@@ -79,6 +79,12 @@ Note that the code above provides a failover mechanism in case of server node fa
include::includes/partition-awareness.adoc[]
+Partition awareness functionality helps to avoid an additional network hop in the following scenarios:
+
+1. Single-key operations API, like put(), get(), etc. However, the functionality has no effect on those operations within explicit transactions (initiated via ClientTransaction.txStart() described in <<Transactions>> section).
+
+2. ScanQuery and IndexQuery accept a partition number as a parameter with which the query is routed to a particular server node that stores the requested data. Refer to <<Executing Scan Queries>> and link:key-value-api/using-cache-queries[Executing Index Queries] sections for more information.
Review Comment:
link:key-value-api/using-cache-queries**#executing-index-queries**
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a diff in pull request #10453: IGNITE-17321 Document which api can work with partition awareness
Posted by "timoninmaxim (via GitHub)" <gi...@apache.org>.
timoninmaxim commented on code in PR #10453:
URL: https://github.com/apache/ignite/pull/10453#discussion_r1131610562
##########
docs/_docs/thin-clients/java-thin-client.adoc:
##########
@@ -79,6 +79,12 @@ Note that the code above provides a failover mechanism in case of server node fa
include::includes/partition-awareness.adoc[]
+Partition awareness functionality helps to avoid an additional network hop in the following scenarios:
+
+1. Single-key operations API, like put(), get(), etc. However, the functionality has no effect on those operations within explicit transactions (initiated via ClientTransaction.txStart() described in <<Transactions>> section).
Review Comment:
.txStart() -> #txStart()
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a diff in pull request #10453: IGNITE-17321 Document which api can work with partition awareness
Posted by "timoninmaxim (via GitHub)" <gi...@apache.org>.
timoninmaxim commented on code in PR #10453:
URL: https://github.com/apache/ignite/pull/10453#discussion_r1131611037
##########
docs/_docs/code-snippets/java/src/main/java/org/apache/ignite/snippets/JavaThinClient.java:
##########
@@ -359,6 +359,9 @@ void partitionAwareness() throws Exception {
try (IgniteClient client = Ignition.startClient(cfg)) {
ClientCache<Integer, String> cache = client.cache("myCache");
// Put, get or remove data from the cache...
+ cache.put(0, "Hello, world!");
+ // The partition number can be specified with indexQuery.setPartition(Integer) as well.
Review Comment:
indexQuery.setPartition(Integer) -> IndexQuery#setPartition(Integer)
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org