IGNITE-17275 Sql. Performance testing of pure SQL execution#1483
Conversation
| * @param <T> A type of the produced elements. | ||
| * @return A data provider instance backed by given collection. | ||
| */ | ||
| static <T> DataProvider<T> fromCollection(Collection<T> collection) { |
There was a problem hiding this comment.
I think this will be used frequently as the framework is being adopted. But if you insist, I'll remove this
There was a problem hiding this comment.
no, it`s all ok in such a case
|
|
||
| /** {@inheritDoc} */ | ||
| @Override | ||
| public ChildT addDataProvider(String targetNode, DataProvider<?> dataProvider) { |
There was a problem hiding this comment.
so, we can have different provider per node? is it useful ? probably we need additional method without targetNode used for all nodes by default ?
There was a problem hiding this comment.
so, we can have different provider per node? is it useful ?
I believe it is very useful. As a framework, it should be flexible enough to give an ability to configure the test cluster in the way it will be configured in real life (assume affinity distribution or data skews)
probably we need additional method without targetNode used for all nodes by default ?
I added ClusterTableBuilder#defaultDataProvider to cover this. Please take a look
| import org.jetbrains.annotations.Nullable; | ||
|
|
||
| /** | ||
| * Dummy cluster service object which helps to created connect |
There was a problem hiding this comment.
| * Dummy cluster service object which helps to created connect | |
| * Dummy cluster service object which helps to create connected |
There was a problem hiding this comment.
Additionally i suppose that too frequent "dummy" word usage is excessive, whole ignite code is hard to understand )
There was a problem hiding this comment.
Paraphrase this a little
| * in execution-related scenarios. | ||
| */ | ||
| public class TestTable implements InternalIgniteTable { | ||
| private static final AssertionError DATA_PROVIDER_NOT_CONFIGURED_EXCEPTION = |
There was a problem hiding this comment.
All that end user can see is just this error creation i dont think its right ?
java.lang.AssertionError: DataProvider not configured at org.apache.ignite.internal.sql.engine.framework.TestTable.<clinit>(TestTable.java:65) at org.apache.ignite.internal.sql.engine.framework.TestBuilders$ClusterTableBuilderImpl.end(TestBuilders.java:167)
There was a problem hiding this comment.
Agree with @zstan,
Let's create DummyProvider that will throw an exception and maybe rewrite
DataProvider<RowT> dataProvider(String nodeName) {
return (DataProvider<RowT>) dataProviders.getOrDefault(nodeName, dummyProvider);
}
There was a problem hiding this comment.
I improved the message by providing the name of the table and name of the node data provider is. requested for
zstan
left a comment
zstan
left a comment
There was a problem hiding this comment.
cool. lets proceed, dont forget about mentioned additional issue.
- change `nodes` method signature to force user to provide at least one node name - add validation that all data providers belongs to a node of the cluster - improve assertion message in case of someone tries to get not configured dataProvider - typos
- add defaultDataProvider configuration to the ClusterTableBuilder
103abb4 to
ffbe15f
Compare
3 participants
The patch introduces the Test Object Builder framework and repas an example of use as very simple test to measure the performance of the query execution engine in isolation from the overhead introduced by tables and transactions