-
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-23554 ScanQuery code cleanup #11645
Conversation
* Checks if transaction mode supported for transaction aware queries. | ||
* @param isolation Transaction isolation to check. | ||
*/ | ||
public static void ensureTransactionModeSupported(TransactionIsolation isolation) { |
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.
Only one usage. Do we really need this method? Can we keep it inlined?
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.
The second usage appear in #11580
try { | ||
Integer part = qry.partition(); | ||
|
||
if (part != null && (part < 0 || part >= cctx.affinity().partitions())) | ||
return new GridEmptyCloseableIterator() { | ||
@Override public void close() throws IgniteCheckedException { | ||
if (intFilter != null) |
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.
Why we don't need to close qry.scanFilter() anymore?
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.
Because, here we don't allocate any resources.
It required and happens only inside ScanQueryIterator
.
ScanQueryIterator
correctly manage those resources inside.
You are correct. Fixed.
@@ -869,36 +852,6 @@ private GridCloseableIterator scanIterator(final CacheQuery<?> qry, IgniteClosur | |||
return iter; | |||
} | |||
catch (IgniteCheckedException | RuntimeException e) { | |||
if (intFilter != null) |
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.
qry.scanFilter() is not closed now (for example, when we cant reserve locPart)
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.
Good catch. Fixed. Thanks.
|
||
/** */ | ||
static void closeFilter(IgniteBiPredicate<?, ?> filter) { | ||
if (filter instanceof PlatformCacheEntryFilter) |
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.
Looks like after sendboxing PlatformCacheEntryFilter
is not instanceof PlatformCacheEntryFilter
anymore
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.
Good catch. Thanks. Fixed.
...e/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java
Show resolved
Hide resolved
.../core/src/main/java/org/apache/ignite/internal/processors/cache/query/ScanQueryIterator.java
Outdated
Show resolved
Hide resolved
f6cb41e
to
896b1c6
Compare
|
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.