-
Notifications
You must be signed in to change notification settings - Fork 10
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
gg-21461 #180
base: ignite-2.5-master
Are you sure you want to change the base?
gg-21461 #180
Conversation
(cherry picked from commit b5f3f72)
(cherry picked from commit 7883609)
(cherry picked from commit 7043f6c)
(cherry picked from commit 1a0fd3e)
…er doesn't configured explicitly. (cherry picked from commit c8bd480)
(cherry-picked from commit #fa8e5da60546bf2e46cc16b13baf0a21c394e7b6)
(cherry picked from commit cafab0d) # Conflicts: # modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/topology/GridDhtPartitionTopologyImpl.java # modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/db/IgnitePdsCacheWalDisabledOnRebalancingTest.java
…ence.GridCacheOffheapManager#restorePartitionStates method. (cherry picked from commit 621f6af)
…) doesn't work. (cherry picked from commit 9350760)
…e-ignite into ignite-2.5-master
…asicIndexSelfTest
…ame make DR paused in case of persistence. (cherry picked from commit e093716)
…ueries. (cherry picked from commit 03283da)
…e same name make DR paused in case of persistence." This reverts commit 3616b6a.
…Left flaky failed on TC (cherry picked from commit fe0ec74)
…ctivateSimple_5_Servers_5_Clients_FromClient flaky failed on TC (cherry picked from commit 2578931)
…ormation about cache group. (cherry picked from commit 82409a9)
…or all x.x-master branches
… ignite system properties (cherry picked from commit 05cdf46)
… called (cherry picked from commit d1b0787)
…ientInForceServerModeStopsOnExchangeHistoryExhaustion is flaky.
… from 8.4.9 to 8.5.7 if POJO primary key is used
(cherry picked from commit 7baa018)
…es and test coverage. Phase #1.
…message, if applicable. (cherry picked from commit 4b045f5)
…about group and cache id. (cherry picked from commit f3017ea)
(cherry picked from commit f7f0b89)
(cherry picked from commit f7f0b89)
(cherry picked from commit 4f05cd9)
(cherry picked from commit fb2553a)
…ce.file.FilePageStore instances (cherry picked from commit 806b1b6)
(cherry picked from commit 0a17bb9)
…p for experemental commands. (cherry picked from commit 86ee0b5)
…pi without IgniteConfiguration.setIncludeEventTypes(EventType.EVT_TASK_FINISHED, EventType.EVT_TASK_FAILED) leads to memory leak - Fixed apache#6690. (apache#293) (cherry picked from commit e2d6632)
(cherry picked from commit d8753ae)
...java/org/apache/ignite/internal/processors/query/h2/database/H2TreeInlineObjectDetector.java
Outdated
Show resolved
Hide resolved
...java/org/apache/ignite/internal/processors/query/h2/database/H2TreeInlineObjectDetector.java
Outdated
Show resolved
Hide resolved
...java/org/apache/ignite/internal/processors/query/h2/database/H2TreeInlineObjectDetector.java
Outdated
Show resolved
Hide resolved
...java/org/apache/ignite/internal/processors/query/h2/database/H2TreeInlineObjectDetector.java
Outdated
Show resolved
Hide resolved
...java/org/apache/ignite/internal/processors/query/h2/database/H2TreeInlineObjectDetector.java
Outdated
Show resolved
Hide resolved
...java/org/apache/ignite/internal/processors/query/h2/database/H2TreeInlineObjectDetector.java
Outdated
Show resolved
Hide resolved
...java/org/apache/ignite/internal/processors/query/h2/database/H2TreeInlineObjectDetector.java
Show resolved
Hide resolved
...java/org/apache/ignite/internal/processors/query/h2/database/H2TreeInlineObjectDetector.java
Outdated
Show resolved
Hide resolved
byte[] originalObjBytes = val.getBytesNoCopy(); | ||
|
||
// read size more then available space or more then origin length | ||
if(len > inlineSize - fieldOff - 3 || len > originalObjBytes.length){ |
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.
len > originalObjBytes.length
seems to mean unexpected (garbage) data in current position. Cannot we do comparison in branch if (type == Value.JAVA_OBJECT)
using following strategy:
- Prepare (virtually) an expected byte content which is saved by regular inline procedures.
- Compare that content with bytes stored in the index page.
- Exact match means that JAVA_OBJECT inline is (almost 100%) supported. Otherwise it is not (tree corruption is possible, might be warning is a good idea). In both cases we can finish tree scan here.
Also using of InlineIndexHelper.compare
is dangerous below, because it is more broad method and it is used in different places. But here we are just solving a compatibility issue from certain point in history.
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 your comparison proposal - by fact it do the same way.
objections to use InlineIndexHelper.compare don't understand. Let's discuss internally.
...java/org/apache/ignite/internal/processors/query/h2/database/H2TreeInlineObjectDetector.java
Show resolved
Hide resolved
@@ -148,7 +167,9 @@ protected H2Tree( | |||
if (metaInfo.useUnwrappedPk()) | |||
throw new IgniteCheckedException("Unwrapped PK is not supported by current version"); | |||
|
|||
this.inlineSize = metaInfo.inlineSize(); | |||
inlineSize = metaInfo.inlineSize(); |
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 my understanding. How is it possible that inlineSize
passed to a constructor is not equal to metaInfo.inlineSize()
?
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.
inlinesize can be different due to use different set of columns to calculate it ( as example before support JAVA_OBJECT type and after)
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.
Where do we calculate it?
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.
@pavlukhin see org.apache.ignite.internal.processors.query.h2.database.H2TreeIndex#computeInlineSize
for (InlineIndexHelper ih : inlineHelpers) { | ||
if (fieldOff >= inlineSize) | ||
return false; | ||
|
||
if (ih.type() != Value.JAVA_OBJECT) { | ||
if(ih.size() < 0) |
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.
Formatting.
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.
yep
...java/org/apache/ignite/internal/processors/query/h2/database/H2TreeInlineObjectDetector.java
Outdated
Show resolved
Hide resolved
byte[] inlineBytes = PageUtils.getBytes(pageAddr, off + fieldOff + 3, len); | ||
|
||
for (int i = 0; i < len; i++) { | ||
if (inlineBytes[i] == originalObjBytes[i]) |
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.
Seems more straightforward to me:
for (int i = 0; i < len; i++) {
if (inlineBytes[i] != originalObjBytes[i]) {
inlineObjectSupportedDecision(false, i + " byte compare");
return true;
}
}
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.
agree
for (InlineIndexHelper ih : inlineHelpers) { | ||
if (fieldOff >= inlineSize) | ||
return false; | ||
|
||
if (ih.type() != Value.JAVA_OBJECT) { | ||
if(ih.size() < 0) | ||
varLenPresents=true; |
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.
I suppose we can initialize it constructor.
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.
yes, but no reason to do it
9f0de85
to
c77bc90
Compare
gg-21461