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
[HZ-771] Align query-cache and near-cache default behavior #20265
Conversation
81bdcb5
to
8aa400b
Compare
96c9bad
to
1adbb38
Compare
1adbb38
to
96a4656
Compare
9fa68ca
to
96af0ca
Compare
b9b2f72
to
048b8e6
Compare
8f04a7c
to
cb376dc
Compare
run-lab-run |
ca625d8
to
84e5597
Compare
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.
LGTM overall, I have small comments around configs and code structure.
hazelcast/src/main/java/com/hazelcast/internal/config/dynamic/DynamicConfigXmlGenerator.java
Show resolved
Hide resolved
...t/src/main/java/com/hazelcast/map/impl/querycache/subscriber/AbstractInternalQueryCache.java
Show resolved
Hide resolved
if (areKeyValueObjectType) { | ||
queryEntry.initWithObjectKeyValue(queryCacheKey, rawValue); | ||
} else { | ||
queryEntry.init(queryCacheKey, rawValue); | ||
} |
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 fail to understand the difference here. What would happen if we were to only call queryEntry.init(queryCacheKey, rawValue);
instead of this if else block?
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, it is an optimization and JMH shows it made some difference positively.initWithObjectKeyValue
has no checks in 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.
Ah, thanks, can you leave a small comment for this then. This way someone like me wouldn't simplify it accidentally.
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.
actually i already did it at the start of method:
// needed for optimization where key and value are not an instance of Data type
final boolean areKeyValueObjectType = !queryCacheConfig.isSerializeKeys()
&& InMemoryFormat.OBJECT == queryCacheConfig.getInMemoryFormat();
...t/src/main/java/com/hazelcast/map/impl/querycache/subscriber/AbstractInternalQueryCache.java
Show resolved
Hide resolved
hazelcast/src/test/java/com/hazelcast/map/impl/querycache/QueryCacheBenchmark.java
Show resolved
Hide resolved
84e5597
to
b404c31
Compare
The job Click to expand the log file-------------------------- -------TEST FAILURE------- -------------------------- [INFO] Results: [INFO] [ERROR] Errors: [ERROR] YamlConfigBuilderTest.testQueryCacheFullConfig:2718->buildConfig:1843 ? SchemaViolationConfiguration [INFO] [ERROR] Tests run: 46689, Failures: 0, Errors: 1, Skipped: 1015 [INFO] |
b404c31
to
8ac584c
Compare
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.
LGTM, I only have one minor comment: #20265 (comment)
closes #19487
Modifications
QueryCache
to eliminate extra deserialization whenOBJECT
in memory format is used.serializeKeys
flag toQueryCacheConfig
to align behavior withNearCache
Docs:
hazelcast/hz-docs#353
Client Protocol Changes: hazelcast/hazelcast-client-protocol#407
Benchmarks
Simulator
test file: hazelcast/hazelcast-simulator#1972
ThisPR(red) vs 5.0.z(blue)
Comparison of
QueryCache#values
throughputJMH
5.0.z
ThisPR