vert-x3 / vertx-ignite

Apache License 2.0
34 stars 28 forks source link

Failed to store session with IgniteClusterManager but succeed with HazelcastClusterManager #60

Closed tsegismont closed 6 years ago

tsegismont commented 6 years ago

Moved from vert-x3/vertx-web#793

Version

Context

hello, when i use IgniteClusterManager , Failed to store session ,but change HazelcastClusterManager,it works right。

with IgniteClusterManager ,excption message is: javax.cache.CacheException: class org.apache.ignite.IgniteCheckedException: Failed to serialize object: {87851d06-3e06-4418-9131-f884123adbf6=io.vertx.core.impl.DeploymentManager$DeploymentImpl@660d9876, 419334ef-cfd6-4a1e-8711-184a8da391b7=io.vertx.core.impl.DeploymentManager$DeploymentImpl@530374f2, 379c09e8-7524-41d4-8800-6c9aeaf50d38=io.vertx.core.impl.DeploymentManager$DeploymentImpl@629b4cf5}

here is my code :

       IgniteClusterManager clusterManager = new IgniteClusterManager();
//        HazelcastClusterManager clusterManager = new HazelcastClusterManager();
        VertxOptions vertxOptions = new VertxOptions().setClusterManager(clusterManager);
        Future<Vertx> vertxFuture = Future.future();

      router.route().handler(CookieHandler.create());
        // create default clustered session
        SessionStore store = ClusteredSessionStore.create(this.vertx);
        SessionHandler sessionHandler = SessionHandler.create(store);
        router.route().handler(sessionHandler);

        Session session = routingContext.session();
        session.put("user", new JsonObject().put("a","a"));
agura commented 6 years ago

@tsegismont I'll take a look.

tsegismont commented 6 years ago

@agura thank you!

agura commented 6 years ago

@tsegismont It's obviously Apache Ignite bug. But why does Vert.x use SessionImpl as cache value? It seems valuable data is only ConcurrentHashMap instance with session attributes.

tsegismont commented 6 years ago

@agura not sure, @pmlopes knows

tsegismont commented 6 years ago

@agura actuall SessionImpl implements ClusterSerializable. So the cluster manager should be able to handle it

tsegismont commented 6 years ago

@agura I just checked, the AsyncMap test suite checks that a Serializable object can be added, but not ClusterSerializable. Handling ClusterSerializable is mandatory. Can Ignite be configured to invoke a special serializer/deserializer for ClusterSerializable?

tsegismont commented 6 years ago

@agura also, I'm going to add a test in AsyncMapTest for ClusterSerializable

agura commented 6 years ago

@tsegismont Ignite doesn't know about Veert.x specific classes. I think Ignite cluster manager should check that key and value implement ClusterSerializable and do proper serialization.

tsegismont commented 6 years ago

@agura I just pushed the new tests in core. When the new snapshot is built, we can overwrite the test to ignore it until this is fixed

agura commented 6 years ago

@tsegismont Thanks!

tsegismont commented 6 years ago

@agura strangely enough, the new tests for ClusterSerializable pass. I wonder how that's possible (the objects used do not implement Serializable). Maybe Ignite does some sort of smart inspection of non-serializable objects. My test class only wraps a String. I'll try with something else.

agura commented 6 years ago

@tsegismont In general, Ignite doesn't require Serializable implementation because it uses proprietary binary marshaller. But there are some cases when standard Java marshaller will be used as fallback marshaler and it can lead to NotSerializableException.

I have a fix for the problem and will commit it soon. It would be great if you will take a look.

tsegismont commented 6 years ago

@agura will do

tsegismont commented 6 years ago

@agura I managed to trick the marshaller so that it fails to serialize the test object. I just pushed 4c6d731 to ignore the tests. When you PR is ready, make sure the tests are executed again.

agura commented 6 years ago

@tsegismont I've committed fix (see #64). Please, take a look.