agura commented on a change in pull request #8135:
URL: https://github.com/apache/ignite/pull/8135#discussion_r471698263
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/managers/systemview/GridSystemViewManager.java
##########
@@ -53,6 +57,9 @@
*/
public class GridSystemViewManager extends
GridManagerAdapter<SystemViewExporterSpi>
implements ReadOnlySystemViewRegistry {
+ /** Class name for a SQL view exporter of system views. */
+ public static final String SYSTEM_VIEW_SQL_SPI =
"org.apache.ignite.internal.managers.systemview.SqlViewExporterSpi";
Review comment:
Visibility scope for this constant is too broad.
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/managers/systemview/GridSystemViewManager.java
##########
@@ -75,7 +82,26 @@
* @param ctx Kernal context.
*/
public GridSystemViewManager(GridKernalContext ctx) {
- super(ctx, ctx.config().getSystemViewExporterSpi());
+ super(ctx, ((Supplier<SystemViewExporterSpi[]>)() -> {
Review comment:
Could you please replace supplier by simple method call? There is no
need to use lambda syntax here.
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/managers/systemview/GridSystemViewManager.java
##########
@@ -75,7 +82,26 @@
* @param ctx Kernal context.
*/
public GridSystemViewManager(GridKernalContext ctx) {
- super(ctx, ctx.config().getSystemViewExporterSpi());
+ super(ctx, ((Supplier<SystemViewExporterSpi[]>)() -> {
+ SystemViewExporterSpi[] spi =
ctx.config().getSystemViewExporterSpi();
+
+ if (!IgniteComponentType.INDEXING.inClassPath())
+ return spi;
+
+ SystemViewExporterSpi[] spiWithSql = new SystemViewExporterSpi[spi
!= null ? spi.length + 1 : 1];
Review comment:
Nevertheless, what if `spi` array already contains `SqlViewExporterSpi`?
##########
File path:
modules/indexing/src/main/java/org/apache/ignite/internal/managers/systemview/SqlViewExporterSpi.java
##########
@@ -36,12 +37,9 @@
* This SPI implementation exports metrics as SQL views.
*
* Note, instance of this class created with reflection.
- * @see IgnitionEx#SYSTEM_VIEW_SQL_SPI
+ * @see GridSystemViewManager#SYSTEM_VIEW_SQL_SPI
*/
-public class SqlViewExporterSpi extends IgniteSpiAdapter implements
SystemViewExporterSpi {
- /** System view filter. */
- @Nullable private Predicate<SystemView<?>> filter;
-
+class SqlViewExporterSpi extends IgniteSpiAdapter implements
SystemViewExporterSpi {
Review comment:
IMHO, it isn't the best solution to have internal implementation of any
SPI. May be we can extract some common parts of Exporter SPI into a new
internal super-class which should be inherited by `SystemViewExporterSpi` and
our internal analog of `SqlViewEXporterSpi`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]