[jira] [Commented] (IGNITE-13383) Java thin client improvements: channel reconnect and redundant concurrency structures replacement.
[
https://issues.apache.org/jira/browse/IGNITE-13383?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17186558#comment-17186558
]
Aleksey Plekhanov commented on IGNITE-13383:
[~zstan], I see no race here. Handshake should always be synchronous from a
clients point of view (client should not send the next message until receiving
handshake response), so it's just impossible to pass condition {{connCtx ==
null}} twice for one session.
If the client implementation is not correct and can send the next message
before receiving handshake response, there will be problems even with
synchronization, since message processing order is not guaranteed, and regular
message can be processed by server before handshake request.
Do I miss something?
> Java thin client improvements: channel reconnect and redundant concurrency
> structures replacement.
> --
>
> Key: IGNITE-13383
> URL: https://issues.apache.org/jira/browse/IGNITE-13383
> Project: Ignite
> Issue Type: Improvement
> Components: thin client
>Affects Versions: 2.8.1
>Reporter: Stanilovsky Evgeny
>Assignee: Stanilovsky Evgeny
>Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> I found that {code:java}ConcurrentHashMap{code} and
> {code:java}AtomicLong{code} are redundant in java thin client code, yes i fix
> tests a bit but i can`t see any contradictions with thick client behavior
> here.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (IGNITE-13383) Java thin client improvements: channel reconnect and redundant concurrency structures replacement.
[
https://issues.apache.org/jira/browse/IGNITE-13383?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17186524#comment-17186524
]
Stanilovsky Evgeny commented on IGNITE-13383:
-
[~alex_pl] i recognize that transparent txId numeration can be implemented such
way, because of changing ClientListenerNioListener and as consequence -
session, the only fix i want to store here is [1] possible race, what do you
think ?
[1]
https://github.com/apache/ignite/pull/8184/files#diff-6f1053ded0ee927b46750b0260159bfcR154
> Java thin client improvements: channel reconnect and redundant concurrency
> structures replacement.
> --
>
> Key: IGNITE-13383
> URL: https://issues.apache.org/jira/browse/IGNITE-13383
> Project: Ignite
> Issue Type: Improvement
> Components: thin client
>Affects Versions: 2.8.1
>Reporter: Stanilovsky Evgeny
>Assignee: Stanilovsky Evgeny
>Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> I found that {code:java}ConcurrentHashMap{code} and
> {code:java}AtomicLong{code} are redundant in java thin client code, yes i fix
> tests a bit but i can`t see any contradictions with thick client behavior
> here.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (IGNITE-13383) Java thin client improvements: channel reconnect and redundant concurrency structures replacement.
[
https://issues.apache.org/jira/browse/IGNITE-13383?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17186341#comment-17186341
]
Stanilovsky Evgeny commented on IGNITE-13383:
-
1. My position - no need to defense from users buggy code on server side,
performance - possible i agree here, but without bench this still unclear.
2. All thin clients need to implement the same.
> Moreover, you introduce memory leak again on the server-side (current
> connection context has reference to the ses - I nullify it, thanks for point.
> and next started transaction will intersect old transactions by id - tests
> are green, i double check them.
> Java thin client improvements: channel reconnect and redundant concurrency
> structures replacement.
> --
>
> Key: IGNITE-13383
> URL: https://issues.apache.org/jira/browse/IGNITE-13383
> Project: Ignite
> Issue Type: Improvement
> Components: thin client
>Affects Versions: 2.8.1
>Reporter: Stanilovsky Evgeny
>Assignee: Stanilovsky Evgeny
>Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> I found that {code:java}ConcurrentHashMap{code} and
> {code:java}AtomicLong{code} are redundant in java thin client code, yes i fix
> tests a bit but i can`t see any contradictions with thick client behavior
> here.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (IGNITE-13383) Java thin client improvements: channel reconnect and redundant concurrency structures replacement.
[
https://issues.apache.org/jira/browse/IGNITE-13383?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17186328#comment-17186328
]
Aleksey Plekhanov commented on IGNITE-13383:
[~zstan], I don't think this ticket should be fixed at all. Both "improvements"
didn't look like improvements.
1. Concurrent structures were introduced for a reason, they prevent memory
leaks (https://github.com/apache/ignite/pull/6734#pullrequestreview-278104733).
Their performance worth nothing compared to other logic (perhaps micropercents
of total request time). Why should we sacrifice correctness for this?
2. What's wrong with the channel check on the client's side? It's a "fail-fast"
approach. With your change client will get the same result, but should send
request to the server to get an error. Moreover, you introduce memory leak
again on the server-side (current connection context has reference to the
{{ses}}, which has reference to the previous connection context, which has
reference to the {{ses}} and so on). And it doesn't protect from txId
intersection. For example, you have 2 clients, both connects to the server,
initial txId for first will be 0, initial txId for the second will be 1, then
first creates three transactions (txId=1, txId=2, txId=3) and lost connection,
after reconnect initial txId will be 2, and next started transaction will
intersect old transactions by id. I think simple and reasonable check on
client-side is better than complex, unclear, and error-prone logic on
server-side.
> Java thin client improvements: channel reconnect and redundant concurrency
> structures replacement.
> --
>
> Key: IGNITE-13383
> URL: https://issues.apache.org/jira/browse/IGNITE-13383
> Project: Ignite
> Issue Type: Improvement
> Components: thin client
>Affects Versions: 2.8.1
>Reporter: Stanilovsky Evgeny
>Assignee: Stanilovsky Evgeny
>Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> I found that {code:java}ConcurrentHashMap{code} and
> {code:java}AtomicLong{code} are redundant in java thin client code, yes i fix
> tests a bit but i can`t see any contradictions with thick client behavior
> here.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (IGNITE-13383) Java thin client improvements: channel reconnect and redundant concurrency structures replacement.
[
https://issues.apache.org/jira/browse/IGNITE-13383?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17186270#comment-17186270
]
Ignite TC Bot commented on IGNITE-13383:
{panel:title=Branch: [pull/8184/head] Base: [master] : No blockers
found!|borderStyle=dashed|borderColor=#ccc|titleBGColor=#D6F7C1}{panel}
{panel:title=Branch: [pull/8184/head] Base: [master] : No new tests
found!|borderStyle=dashed|borderColor=#ccc|titleBGColor=#F7D6C1}{panel}
[TeamCity *-- Run :: All*
Results|https://ci.ignite.apache.org/viewLog.html?buildId=5566036buildTypeId=IgniteTests24Java8_RunAll]
> Java thin client improvements: channel reconnect and redundant concurrency
> structures replacement.
> --
>
> Key: IGNITE-13383
> URL: https://issues.apache.org/jira/browse/IGNITE-13383
> Project: Ignite
> Issue Type: Improvement
> Components: thin client
>Affects Versions: 2.8.1
>Reporter: Stanilovsky Evgeny
>Assignee: Stanilovsky Evgeny
>Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> I found that {code:java}ConcurrentHashMap{code} and
> {code:java}AtomicLong{code} are redundant in java thin client code, yes i fix
> tests a bit but i can`t see any contradictions with thick client behavior
> here.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
