IGNITE-24221 Implement new benchmarks that cover creating a distribution zone and table #5081
Conversation
| @@ -90,25 +90,35 @@ public void nodeSetUp() throws Exception { | |||
| startCluster(); | |||
|
|
|||
| try { | |||
| var queryEngine = igniteImpl.queryEngine(); | |||
| // Create a default zone on the cluster's start-up. | |||
| createDefaultZoneOnStartup(); | |||
There was a problem hiding this comment.
It seems there is a clash in terminology. There is a default zone that is created by the cluster on initialization automatically (implicitly). Here, another zone is created explicitly, so it probably should not be named a 'default zone'. How about just createZoneOnStartup()?
| var createZoneStatement = "CREATE ZONE IF NOT EXISTS " + ZONE_NAME + " WITH partitions=" + partitionCount() | ||
| + ", replicas=" + replicaCount() + ", storage_profiles ='" + DEFAULT_STORAGE_PROFILE + "'"; | ||
|
|
||
| getAllFromCursor( |
There was a problem hiding this comment.
I wonder why we don't create a zone via public SQL API. Does it make sense to ask the guys who wrote this initially, and if there is no good reason for this, to switch to public API usage? It would make it more difficult to break something later accidentally
| private int replicaCount; | ||
|
|
||
| /** Distribution zones counter. */ | ||
| private AtomicInteger cnt = new AtomicInteger(); |
There was a problem hiding this comment.
| private AtomicInteger cnt = new AtomicInteger(); | |
| private final AtomicInteger cnt = new AtomicInteger(); |
| @OutputTimeUnit(MILLISECONDS) | ||
| public void createEmptyDistributionZone() { | ||
| ZoneDefinition zone = ZoneDefinition.builder("zone_test_" + cnt.incrementAndGet()) | ||
| .ifNotExists() |
There was a problem hiding this comment.
Why do we need this? Could it mask a programming error if we try to create a zone that already exists?
| */ | ||
| @Fork(1) | ||
| @State(Scope.Benchmark) | ||
| public class CreatingDistributionZoneBenchmark extends AbstractMultiNodeBenchmark { |
There was a problem hiding this comment.
AbstractMultiNodeBenchmark inits cluster with TestIgnitionManager.init(), which uses test defaults for things like delayDuration, idleSafeTimePropagationInterval, maxClockSkew. Test defaults for these values are ridiculously low; benchmarking with them has some value as it allows to (almost) exclude waits imposed by the schema sync protocol.
But maybe we also need to benchmark with real defaults? There is a crutch: you can pass TestIgnitionManager#PRODUCTION_CLUSTER_CONFIG_STRING as cluster config to instruct TestIgnitionManager#init() to NOT apply test defaults.
Maybe we could have a boolean parameter in the benchmark, like tinySchemaSyncWaits? If it is true, we could keep current behavior; otherwise, we could use production defaults to make schema sync look real.
| public void createTableInDefaultZone() { | ||
| String tableName = "table_test_" + cnt.incrementAndGet(); | ||
|
|
||
| createTable(tableName); |
There was a problem hiding this comment.
This method should be switched to public API as otherwise it's not clear what we measure here
| @Measurement(iterations = 5, time = 5) | ||
| @BenchmarkMode(AverageTime) | ||
| @OutputTimeUnit(MILLISECONDS) | ||
| public void createTableInDefaultZone() { |
There was a problem hiding this comment.
How about having a boolean parameter that would tell whether a put should be made or not? We would be able to see the gap between creating an empty table and it becoming ready for puts.
| */ | ||
| @Fork(1) | ||
| @State(Scope.Benchmark) | ||
| public class CreatingTableBenchmark extends AbstractMultiNodeBenchmark { |
There was a problem hiding this comment.
Same thing about test defaults
https://issues.apache.org/jira/browse/IGNITE-24221
Creating a new distribution zone:
Creating a new table in the default distribution zone:
Thank you for submitting the pull request.
To streamline the review process of the patch and ensure better code quality
we ask both an author and a reviewer to verify the following:
The Review Checklist
- There is a single JIRA ticket related to the pull request.
- The web-link to the pull request is attached to the JIRA ticket.
- The JIRA ticket has the Patch Available state.
- The description of the JIRA ticket explains WHAT was made, WHY and HOW.
- The pull request title is treated as the final commit message. The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
Notes