Perform resource existence checks only once in Mongo connector [HZ-2673] #24953
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TomaszGaweda
added
Type: Enhancement
Source: Internal
6 tasks
TomaszGaweda
changed the title
k-jamroz
reviewed
Jul 5, 2023
extensions/mongodb/src/main/java/com/hazelcast/jet/mongodb/impl/DbCheckingPMetaSupplier.java
Show resolved
Hide resolved
k-jamroz
reviewed
Jul 5, 2023
extensions/mongodb/src/main/java/com/hazelcast/jet/mongodb/impl/DbCheckingPMetaSupplier.java
Outdated
Show resolved
Hide resolved
k-jamroz
reviewed
Jul 5, 2023
extensions/mongodb/src/main/java/com/hazelcast/jet/mongodb/impl/DbCheckingPMetaSupplier.java
Outdated
Show resolved
Hide resolved
k-jamroz
reviewed
Jul 5, 2023
extensions/mongodb/src/main/java/com/hazelcast/jet/mongodb/impl/DbCheckingPMetaSupplier.java
Outdated
Show resolved
Hide resolved
k-jamroz
reviewed
Jul 5, 2023
extensions/mongodb/src/main/java/com/hazelcast/jet/mongodb/impl/DbCheckingPMetaSupplier.java
Outdated
Show resolved
Hide resolved
k-jamroz
reviewed
Jul 5, 2023
extensions/mongodb/src/main/java/com/hazelcast/jet/mongodb/impl/DbCheckingPMetaSupplier.java
Outdated
Show resolved
Hide resolved
k-jamroz
reviewed
Sep 15, 2023
extensions/mongodb/src/main/java/com/hazelcast/jet/mongodb/MongoSourceBuilder.java
Outdated
Show resolved
Hide resolved
k-jamroz
reviewed
Sep 15, 2023
extensions/mongodb/src/main/java/com/hazelcast/jet/mongodb/MongoSourceBuilder.java
Outdated
Show resolved
Hide resolved
k-jamroz
reviewed
Sep 15, 2023
extensions/mongodb/src/main/java/com/hazelcast/jet/mongodb/ResourceChecks.java
Outdated
Show resolved
Hide resolved
k-jamroz
reviewed
Sep 15, 2023
extensions/mongodb/src/main/java/com/hazelcast/jet/mongodb/ResourceChecks.java
Outdated
Show resolved
Hide resolved
k-jamroz
reviewed
Sep 15, 2023
extensions/mongodb/src/main/java/com/hazelcast/jet/mongodb/impl/ReadMongoP.java
Show resolved
Hide resolved
k-jamroz
reviewed
Sep 15, 2023
extensions/mongodb/src/test/java/com/hazelcast/jet/mongodb/MongoSourceTest.java
Outdated
Show resolved
Hide resolved
k-jamroz
reviewed
Sep 15, 2023
extensions/mongodb/src/main/java/com/hazelcast/jet/mongodb/ResourceChecks.java
Outdated
Show resolved
Hide resolved
k-jamroz
reviewed
Sep 15, 2023
hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/connector/mongodb/MongoTable.java
Outdated
Show resolved
Hide resolved
k-jamroz
reviewed
Sep 15, 2023
...st-sql/src/test/java/com/hazelcast/jet/sql/impl/connector/mongodb/MongoSqlConnectorTest.java
Outdated
Show resolved
Hide resolved
k-jamroz
reviewed
Sep 15, 2023
extensions/mongodb/src/main/java/com/hazelcast/jet/mongodb/MongoSourceBuilder.java
Outdated
Show resolved
Hide resolved
|
The job Click to expand the log file-------------------------- ---------SUMMARY---------- -------------------------- [ERROR] COMPILATION ERROR : -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/test/java/com/hazelcast/jet/sql/impl/index/HDIndexRangeFilterIteratorTest.java:[19,52] error: cannot find symbol -------------------------- ---------ERRORS----------- -------------------------- [ERROR] COMPILATION ERROR : -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/test/java/com/hazelcast/jet/sql/impl/index/HDIndexRangeFilterIteratorTest.java:[19,52] error: cannot find symbol -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/test/java/com/hazelcast/jet/sql/impl/index/HDIndexInFilterIterationTest.java:[19,50] error: cannot find symbol -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/test/java/com/hazelcast/jet/sql/impl/index/HDIndexEqualsFilterIterationTest.java:[19,54] error: cannot find symbol -------------------------- [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.11.0:testCompile (default-testCompile) on project hazelcast-enterprise: Compilation failure: Compilation failure: -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/test/java/com/hazelcast/jet/sql/impl/index/HDIndexRangeFilterIteratorTest.java:[19,52] error: cannot find symbol -------------------------- [ERROR] symbol: class IndexRangeFilterIteratorTest -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/test/java/com/hazelcast/jet/sql/impl/index/HDIndexInFilterIterationTest.java:[19,50] error: cannot find symbol -------------------------- [ERROR] symbol: class IndexCompositeFilterIterationTest -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/test/java/com/hazelcast/jet/sql/impl/index/HDIndexEqualsFilterIterationTest.java:[19,54] error: cannot find symbol -------------------------- [ERROR] symbol: class IndexEqualsFilterIterationTest -------------------------- [ERROR] -> [Help 1] -------------------------- [ERROR] -------------------------- [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch. -------------------------- [ERROR] Re-run Maven using the -X switch to enable full debug logging. -------------------------- [ERROR] -------------------------- [ERROR] For more information about the errors and possible solutions, please read the following articles: -------------------------- [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException -------------------------- [ERROR] -------------------------- [ERROR] After correcting the problems, you can resume the build with the command -------------------------- [ERROR] mvn -rf :hazelcast-enterprise -------------------------- |
k-jamroz
reviewed
Sep 15, 2023
hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/connector/mongodb/MongoTable.java
Outdated
Show resolved
Hide resolved
k-jamroz
reviewed
Sep 15, 2023
extensions/mongodb/src/main/java/com/hazelcast/jet/mongodb/MongoSinkBuilder.java
Outdated
Show resolved
Hide resolved
k-jamroz
reviewed
Sep 15, 2023
extensions/mongodb/src/main/java/com/hazelcast/jet/mongodb/MongoSinkBuilder.java
Show resolved
Hide resolved
k-jamroz
reviewed
Sep 19, 2023
| @@ -200,59 +200,29 @@ public static MongoSourceBuilder.Stream<Document> stream( | |||
| return stream("MongoStreamSource(" + dataConnectionRef.getName() + ")", dataConnectionRef); | |||
| } | |||
|
|
|||
| private abstract static class Base<T> { | |||
| @SuppressWarnings("unchecked") | |||
There was a problem hiding this comment.
nit: usually done with
SELF self() { return (SELF)this; }
k-jamroz
reviewed
Sep 19, 2023
| @Nonnull SupplierEx<? extends MongoClient> clientSupplier | ||
| ) { | ||
| checkSerializable(clientSupplier, "clientSupplier"); | ||
| this.name = name; |
There was a problem hiding this comment.
nit: some of that code could be moved to superclass constructor
k-jamroz
reviewed
Sep 19, 2023
| @@ -412,10 +410,20 @@ public BatchSource<T> build() { | |||
| checkNotNull(params.getMapItemFn(), "mapFn must be set"); | |||
|
|
|||
| final ReadMongoParams<T> localParams = params; | |||
There was a problem hiding this comment.
what is the point of having localParams variable? ReadMongoParams is mutable and ReadMongoParams.setXXX methods do not return new instance.
I can see that it is used in lambda in withProcessorSupplier but all sources created from the same builder will share the same instance of params. If the builder is modified after build() you can have problems (local variable fixes compilation problem but not the sharing problem)
There was a problem hiding this comment.
treating builder as a prototype might not be a best idea but someone might try do that
There was a problem hiding this comment.
usage of localParams throughout this methods makes me think that maybe you wanted to clone them
There was a problem hiding this comment.
we have similar situation in MongoSinkBuilder
There was a problem hiding this comment.
Referencing fields of the builder inside lambda will catch the reference to the builder, meaning that whole builder would need to become serializable. Referencing local variable instead solves the issue.
There was a problem hiding this comment.
yes, but it does not solve problem with builder as prototype. We do not control lifecycle of the builder and the time when PMS will be serialized.
k-jamroz
approved these changes
Sep 19, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead of performing check on every processor, do it once.
Additionally, make it possible to disable the checks in SQL connector.
Possible value of new flag "checkExistence":
only-initial: db/collection existence will be performed once, during mapping creation.once-per-job: as above + during every queryon-each-connect- similar to above, but in case of reconnection (e.g. restore after failure) it will perform the checks again.never- well... never.Fixes performance issue found during benchmarking, where checks were 1/3 of execution time.
Breaking changes:
throwOnNonExistingflags were changed tocheckResourceExistencewith enum values instead of true/falseforceMongoReadParallelismOnewas changed toforceReadTotalParallelismOneChecklist:
Team:,Type:,Source:,Module:) and Milestone setAdd to Release NotesorNot Release Notes contentset