-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IGNITE-22733 Support transactions in Scan query #11580
Conversation
|
...es/indexing/src/test/java/org/apache/ignite/testsuites/IgniteBinaryCacheQueryTestSuite4.java
Outdated
Show resolved
Hide resolved
@@ -147,6 +152,10 @@ public class GridCacheQueryRequest extends GridCacheIdMessage implements GridCac | |||
/** */ | |||
private AffinityTopologyVersion topVer; | |||
|
|||
/** Set of keys that must be skiped during iteration. */ | |||
@GridDirectCollection(KeyCacheObject.class) | |||
private Collection<KeyCacheObject> skipKeys; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it worth to filter keys on reducer instead of modifying network protocol. I think it will be a little bit simplier. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are several reason to filter keys on remote nodes:
- Reduce network usage. We don't send values that will be filtered out.
- In case transformer used, we invoke it on remote node and receive transformed value on local node. This reduce network usage, also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- For example, you have 2 skipKeys and 10 nodes, in case of filtering on remote nodes you send additionaly 2 * 9 keys and receive 2 keys and 2 values less (but receive the same amount in case of insert only transactions). So, we can't be sure about reducing network usage. I think, in the typical usage, on the contrary, it will increase network usage.
- Transformers can enlarge values as well.
* @return First, set of object changed in transaction, second, list of transaction data in required format. | ||
* @see ExecutionContext#transactionChanges(int, int[], Function) | ||
*/ | ||
public IgniteBiTuple<Set<KeyCacheObject>, List<Object>> transactionChanges(Integer part) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a lot of declaration like IgniteBiTuple<Set<KeyCacheObject>, List<Object>>
now in code, and a lot of calls like txChanges.get1()
, txChanges.get2()
. Perhaps we are ready to introduce new class for query transactional changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
...apache/ignite/internal/processors/cache/query/ScanQueryTransactionsUnsupportedModesTest.java
Outdated
Show resolved
Hide resolved
...rc/test/java/org/apache/ignite/internal/processors/cache/AbstractTransactionalQueryTest.java
Outdated
Show resolved
Hide resolved
...ava/org/apache/ignite/internal/processors/cache/query/ScanQueryTransactionIsolationTest.java
Outdated
Show resolved
Hide resolved
...ava/org/apache/ignite/internal/processors/cache/query/ScanQueryTransactionIsolationTest.java
Outdated
Show resolved
Hide resolved
...ava/org/apache/ignite/internal/processors/cache/query/ScanQueryTransactionIsolationTest.java
Outdated
Show resolved
Hide resolved
try { | ||
for (int i = 0; i < 50; i++) | ||
cache.put(i, new Value("str" + i, i * 100)); | ||
assumeTrue(sqlTxMode == TestTransactionMode.NONE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any transactional test with local flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
} | ||
|
||
/** | ||
* @throws Exception If failed. | ||
*/ | ||
@Test | ||
public void testGetObjectFieldPartitioned() throws Exception { | ||
IgniteCache<Integer, Value> cache = grid().createCache("test-cache"); | ||
IgniteCache<Integer, Value> cache = createTestCache(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test only fill one partition, I think for transactional mode we should also check that tx entries with are filtered correcly by partition if there are data for other partitions exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
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
The description explains WHAT and WHY was made instead of HOW.
The following pattern must be used:
IGNITE-XXXX Change summary
whereXXXX
- number of JIRA issue.(see the Maintainers list)
the
green visa
attached to the JIRA ticket (see TC.Bot: Check PR)Notes
If you need any help, please email [email protected] or ask anу advice on http://asf.slack.com #ignite channel.