alex-plekhanov commented on code in PR #11951:
URL: https://github.com/apache/ignite/pull/11951#discussion_r2010558644
##########
modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java:
##########
@@ -1524,7 +1511,7 @@ private void suggestOptimizations(IgniteConfiguration
cfg) {
if (cfg.getIncludeEventTypes() != null &&
cfg.getIncludeEventTypes().length != 0)
perf.add("Disable grid events (remove 'includeEventTypes' from
configuration)");
- if (BinaryMarshaller.available() && (cfg.getMarshaller() != null &&
!(cfg.getMarshaller() instanceof BinaryMarshaller)))
+ if (BinaryMarshaller.available() && (ctx.marshaller() != null &&
!(ctx.marshaller() instanceof BinaryMarshaller)))
Review Comment:
Looks like these checks are useless, since BinaryMarshaller is only possible
marshaller in context (perhaps we can even return BinaryMarshaller class
instead of Marshaller class).
Also, `do not set 'marshaller' explicitly` message is confusing, we can't
set mashaller explicetly now.
Also, there should be critical error, not performance warning in case
`BinaryMarshaller.available() == false`, since we can't use and configure
another marshaller. Or we should create `jdkMarshaller` implicetely instead of
binary marshaller in ctx, when binary marshaller is not available (but we have
not tests for this case now).
##########
modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentLocalStore.java:
##########
@@ -576,8 +576,8 @@ private void undeploy(ClassLoader ldr) {
ctx.resource().onUndeployed(dep);
// Clear optimized marshaller's cache.
- if (ctx.config().getMarshaller() instanceof AbstractMarshaller)
-
((AbstractMarshaller)ctx.config().getMarshaller()).onUndeploy(ldr);
+ if (ctx.marshaller() instanceof AbstractMarshaller)
Review Comment:
Change class of `ctx.marshaller()` to `BinaryMarshaller`?
##########
modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentPerVersionStore.java:
##########
@@ -1331,8 +1331,8 @@ void recordUndeployed(@Nullable UUID leftNodeId) {
U.clearClassFromClassCache(ctx.cache().context().deploy().globalLoader(),
alias);
// Clear optimized marshaller's cache.
- if (ctx.config().getMarshaller() instanceof AbstractMarshaller)
-
((AbstractMarshaller)ctx.config().getMarshaller()).onUndeploy(ldr);
+ if (ctx.marshaller() instanceof AbstractMarshaller)
Review Comment:
Change class of `ctx.marshaller()` to `BinaryMarshaller`?
##########
modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java:
##########
@@ -1614,12 +1601,12 @@ private void fillNodeAttributes(boolean notifyEnabled)
throws IgniteCheckedExcep
add(ATTR_JIT_NAME, U.getCompilerMx() == null ? "" :
U.getCompilerMx().getName());
add(ATTR_BUILD_VER, VER_STR);
add(ATTR_BUILD_DATE, BUILD_TSTAMP_STR);
- add(ATTR_MARSHALLER, cfg.getMarshaller().getClass().getName());
+ add(ATTR_MARSHALLER, ctx.marshaller().getClass().getName());
add(ATTR_MARSHALLER_USE_DFLT_SUID,
getBoolean(IGNITE_OPTIMIZED_MARSHALLER_USE_DEFAULT_SUID,
OptimizedMarshaller.USE_DFLT_SUID));
add(ATTR_LATE_AFFINITY_ASSIGNMENT, cfg.isLateAffinityAssignment());
- if (cfg.getMarshaller() instanceof BinaryMarshaller) {
+ if (ctx.marshaller() instanceof BinaryMarshaller) {
Review Comment:
Always true
##########
modules/platforms/dotnet/Apache.Ignite.Core.Tests/Config/marshaller-explicit.xml:
##########
@@ -31,10 +31,6 @@
</bean>
</property>
- <property name="marshaller">
- <bean class="org.apache.ignite.internal.binary.BinaryMarshaller"/>
- </property>
-
Review Comment:
Do we need this file at all? It's only used in TestExplicitMarshaller, looks
like now this test can be deleted.
##########
modules/core/src/main/java/org/apache/ignite/internal/managers/deployment/GridDeploymentPerLoaderStore.java:
##########
@@ -517,8 +517,8 @@ void recordUndeployed() {
ctx.cache().onUndeployed(ldr);
// Clear optimized marshaller's cache.
- if (ctx.config().getMarshaller() instanceof AbstractMarshaller)
-
((AbstractMarshaller)ctx.config().getMarshaller()).onUndeploy(ldr);
+ if (ctx.marshaller() instanceof AbstractMarshaller)
Review Comment:
Change class of `ctx.marshaller()` to `BinaryMarshaller`?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
