[jira] [Commented] (IGNITE-12921) System views design leads to bad user expirience.

["Andrey N. Gura (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/IGNITE-12921?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17196592#comment-17196592 ] 

Andrey N. Gura commented on IGNITE-12921:
-----------------------------------------

[~nizhikov] LGTM. Please proceed with merge. Thanks for this change.

> System views design leads to bad user expirience.
> -------------------------------------------------
>
>                 Key: IGNITE-12921
>                 URL: https://issues.apache.org/jira/browse/IGNITE-12921
>             Project: Ignite
>          Issue Type: Bug
>    Affects Versions: 2.8
>            Reporter: Andrey N. Gura
>            Assignee: Nikolay Izhikov
>            Priority: Critical
>              Labels: IEP-35
>             Fix For: 2.10
>
>          Time Spent: 1.5h
>  Remaining Estimate: 0h
>
> Current implementation of system views has broken system behavior which is related with querying system views. 
> Before 2.8 system views were available via SQL queries (if indexing is enabled). It did not depend on any configuration. 
> After implementation of IGNITE-12145 system views available only if {{SqlViewExporterSpi}} is passed to {{IgniteConfiguration.setSystemViewExporterSpi()}}. Now, if an user configures some {{SystemViewExporterSpi}} then provided user configuration will rewrite default configuration and {{SqlViewExporterSpi}} won't be initialized. As result it is impossible to query system views and any query to the views fails with exception. This behavior is not obvious for the user. See tests below.
> The second problem is kind of design problem. System view is internal part of the system and should be available regardless of any exporter configuration (at least via SQL) such as it was implemented before 2.8 release. 
> My suggestion is the following: we should remove {{SqlViewExporterSpi}} and configure all views on indexing module initialization. {{SqlViewExporterSPI}} also doesn't make sense because:
> - it operates by some internal API ({{SchemaManager}}, {{GridKernalContext}}, {{IgniteH2Indexing}}).
> - it doesn't allow to end user to add any new system view.
> Only thing that could be useful is a filtering. But it could be done with SQL.
> Reproducer of broken behavior:
> {code:java}
> package org.apache.ignite.internal.processors.cache.metric;
> import org.apache.ignite.cache.query.SqlFieldsQuery;
> import org.apache.ignite.cluster.ClusterState;
> import org.apache.ignite.configuration.DataRegionConfiguration;
> import org.apache.ignite.configuration.DataStorageConfiguration;
> import org.apache.ignite.configuration.IgniteConfiguration;
> import org.apache.ignite.internal.IgniteEx;
> import org.apache.ignite.spi.systemview.jmx.JmxSystemViewExporterSpi;
> import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
> import org.junit.Test;
> import java.util.HashSet;
> import java.util.List;
> import java.util.Set;
> import static java.util.Arrays.asList;
> import static org.apache.ignite.internal.processors.cache.index.AbstractSchemaSelfTest.queryProcessor;
> public class SystemViewTest extends GridCommonAbstractTest {
>     private static boolean useDefaultSpi;
>     /** {@inheritDoc} */
>     @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
>         IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
>         cfg.setConsistentId(igniteInstanceName);
>         cfg.setDataStorageConfiguration(new DataStorageConfiguration()
>                 .setDataRegionConfigurations(
>                         new DataRegionConfiguration().setName("in-memory").setMaxSize(100L * 1024 * 1024))
>                 .setDefaultDataRegionConfiguration(
>                         new DataRegionConfiguration()
>                                 .setPersistenceEnabled(true)));
>         if (!useDefaultSpi) {
>             // Configure user provided system view exporter SPI.
>             cfg.setSystemViewExporterSpi(new JmxSystemViewExporterSpi());
>         }
>         return cfg;
>     }
>     /**
>      * Will executed succefully.
>      */
>     @Test
>     public void testSystemViewWithDefaultSpi() throws Exception {
>         useDefaultSpi = true;
>         doTestSystemView();
>     }
>     /**
>      * Will fail with <code>Table "VIEWS" not found</code>.
>      */
>     @Test
>     public void testSystemViewWithCustomSpi() throws Exception {
>         useDefaultSpi = false;
>         doTestSystemView();
>     }
>     private void doTestSystemView() throws Exception {
>         try (IgniteEx ignite = startGrid()) {
>             ignite.cluster().state(ClusterState.ACTIVE);
>             Set<String> cacheNames = new HashSet<>(asList("cache-1", "cache-2"));
>             for (String name : cacheNames)
>                 ignite.getOrCreateCache(name);
>             SqlFieldsQuery qry = new SqlFieldsQuery("SELECT * FROM SYS.VIEWS");
>             List<List<?>> res = queryProcessor(ignite).querySqlFields(qry, true).getAll();
>             res.forEach(item -> log.info("VIEW FOUND: " + item));
>         }
>     }
> }
> {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)