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

Ivan Bessonov commented on IGNITE-11397:
----------------------------------------

Hello [~uday], thank you for the contribution! I'd like to discuss few things 
here.

First of all, these tests failures look very suspicious, at least in "Data 
Structures" suite. Please take a look at them.

 

Now more specific. In modified abstract test you have this line:
{code:java}
private static final boolean BINARY_SET_MODE = 
IgniteSystemProperties.getBoolean(BINARY_SET, true);{code}
You set "true" as a default value and you also set "true" in suite. I can't 
find a place where you would use "false" as a value for this property, please 
correct that.

 
{code:java}
public <V1> IgniteSet<V1> withKeepBinary();{code}
I understand where this type of method signature is coming from, but I'd like 
to be sure that it's correct. There are two implementations of this method and 
they have following lines in them:
{code:java}
return new GridCacheSetProxy<>(ctx, (GridCacheSetImpl<V1>)this);{code}
{code:java}
return new GridCacheSetProxy<>(cctx, (GridCacheSetImpl<V1>)delegate);{code}
Both "this" and "delegate" are instances of "IgniteSet<T>" so why do we need a 
new type "V1" that is not even bound with anything? Looks problematic to say 
the least. On the other hand, actual type when operating these sets is most 
likely a "BinaryObject". If it is then we could reflect this fact in method 
signature so it won't be overly complicated.

 

Finally, you mishandled "CacheOperationContext". Please take a look at 
"GatewayProtectedCacheProxy" class and usages of "operationContextPerCall" 
method. What you actually need is to have "opCtx" field in "GridCacheSetProxy" 
and pass it into the gate. Please don't forget to restore previous values on 
leaving the gate, like, for example, in 
"GatewayProtectedCacheProxy.CacheOperationGate" class usages.

Here is the simple reproducer for the problem. It doesn't cover all the aspects 
but is enough for the start:
{code:java}
@Test
public void testGet() throws Exception {
    IgniteEx ignite = grid(0);

    IgniteQueue<BinaryObject> queue = ignite.queue("q0", 10, 
config(false)).withKeepBinary();

    queue.add(sameHashBinObj(ignite, 0));
    queue.add(sameHashBinObj(ignite, 1));

    BinaryObject o0 = queue.poll(); // Ok.

    BinaryObject o1 = 
GridTestUtils.runAsync((Callable<BinaryObject>)queue::poll).get(); // Fails.
}
{code}
You probably noticed that this test operates with "IgniteQueue". It has all the 
same issues so they can be reproduced in master. I would appreciate if you 
created another issue to fix these bugs in "IgniteQueue" after we finish 
"IgniteSet" improvements.

 

Please feel free asking me more questions if you have some. Thank you!

> Binary mode for Ignite Set
> --------------------------
>
>                 Key: IGNITE-11397
>                 URL: https://issues.apache.org/jira/browse/IGNITE-11397
>             Project: Ignite
>          Issue Type: New Feature
>          Components: binary, data structures
>            Reporter: Uday Kale
>            Assignee: Uday Kale
>            Priority: Major
>             Fix For: 2.8
>
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> Add binary mode (withKeepBinary) to Data Structures Set.
> This will allow retrieving values in binary format when needed or when class 
> is not available, and will allow implementing the structures in other 
> platforms (.NET, C++).



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to