-
Notifications
You must be signed in to change notification settings - Fork 988
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
Load Tests - Cassandra Reverse Replication #2163
base: main
Are you sure you want to change the base?
Load Tests - Cassandra Reverse Replication #2163
Conversation
Sync master
Sync main branch
Sync main branch
tests: Adding Forward Migration Tests (GoogleCloudPlatform#2001)
Sync main branch
Use [self-hosted, it] for prepare java cache workflow (GoogleCloudPlatform#2080)
Sync main branch
Sync main branch
Sync main branch
Sync main branch
Metadata config and pipeline options (GoogleCloudPlatform#2081)
fix: eager source row fetching logic (GoogleCloudPlatform#2071)
sync upstream/main
Sync main with upstream
Sync main branch
Support avro arrays for postgres insertion. (GoogleCloudPlatform#2154)
* Addition of Load Tests in SpannerToSourceDB For Cassandra
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2163 +/- ##
=========================================
Coverage 46.89% 46.90%
- Complexity 4346 4351 +5
=========================================
Files 874 874
Lines 52070 52090 +20
Branches 5461 5468 +7
=========================================
+ Hits 24420 24433 +13
- Misses 25906 25909 +3
- Partials 1744 1748 +4
|
...-sourcedb/src/test/java/com/google/cloud/teleport/v2/templates/SpannerToCassandraLTBase.java
Show resolved
Hide resolved
...ner-to-sourcedb/src/test/java/com/google/cloud/teleport/v2/templates/CassandraRowsCheck.java
Outdated
Show resolved
Hide resolved
throw new IllegalArgumentException("CassandraResourceManager must not be null."); | ||
} | ||
|
||
// Use reflection to call executeStatement if it 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.
Why is reflection needed? Can you give some context?
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.
@darshan-sj as resourceManager object's type (T) is generic, and the method executeStatement is not part of a well-defined interface or superclass that resourceManager is guaranteed to implement. Reflection allows here to dynamically invoke the executeStatement method on the resourceManager object if available
*/ | ||
public synchronized ResultSet executeStatement(String statement) { | ||
LOG.info("Executing statement: {}", statement); | ||
return this.executeStatement(statement, DEFAULT_CASSANDRA_TIMEOUT); |
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.
We are introducing a 2 second timeout on this default executeStatement method. We should not modify the behavior for other tests which are using this method.
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.
@darshan-sj I guess request timeout is 2 sec only so does not required any changes for other method
import org.apache.beam.it.gcp.storage.GcsResourceManager; | ||
|
||
/** | ||
* Base class for Spanner to sourcedb Load tests. It provides helper functions related to |
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.
Correct the comment also to reflect Spanner to Cassandra Load tests
.
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.
Addressed @darshan-sj
private static final String TEMPLATE_SPEC_PATH = | ||
MoreObjects.firstNonNull( | ||
TestProperties.specPath(), | ||
"gs://dataflow-templates-spanner-to-cassandra/templates/flex/Spanner_to_SourceDb"); |
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 is this bucket gs://dataflow-templates-spanner-to-cassandra/
?
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.
Addressed @darshan-sj
MoreObjects.firstNonNull( | ||
TestProperties.specPath(), | ||
"gs://dataflow-templates-spanner-to-cassandra/templates/flex/Spanner_to_SourceDb"); | ||
public CassandraResourceManager cassandraSharedResourceManager; |
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 is this variable called `SharedResourceManager? How is it shared?
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.
Addressed @darshan-sj it is typo mistake
getGcsPath(artifactBucket, "input/shard.json", gcsResourceManager)); | ||
getGcsPath( | ||
artifactBucket, | ||
!Objects.equals(sourceType, MYSQL_SOURCE_TYPE) |
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.
Please use template parameters overriding instead of having a ternary operator here.
Example:
Line 152 in b51c933
params.putAll(templateParameters); |
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.
@darshan-sj address the same
This PR introduces Load Testing for the Spanner to Cassandra DB pipeline to evaluate performance, and reliability under high data loads.
Key Changes: