[
https://issues.apache.org/jira/browse/IGNITE-12853?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17074779#comment-17074779
]
Aleksey Plekhanov commented on IGNITE-12853:
--------------------------------------------
[~isapego], I've reviewed your patch briefly:
# I think it's redundant to add methods to
{{ProtocolContext}}/{{ClientProtocolContext}} for each feature.
# I think it's redundant to add methods to
{{ProtocolContext}}/{{ClientProtocolContext}} for old features that were bonded
to protocol versions. At least at the client-side frequently usage pattern: if
ver < xxx then error(version should be xxx), so in any way, we bounded to
protocol version in the message, so I think checking the version and requiring
this version in the message was more intuitive here.
# Class {{ProtocolFeatures}} seems redundant too. Java client always supports
only a subset of features that support the server from the same release, why do
we need to duplicate it? If it was made for a clear separation of server-side
and client-side classes, but {{ProtocolFeatures}} still implements the
server-side interface.
So, code for one feature should be duplicated 4 times. I propose to remove all
{{is...Supported()}} methods from {{ProtocolContext}}/{{ClientProtocolContext}}
and left only two methods: {{version()}} and
{{isFeatureSupported(ThinProtocolFeature)}}, return checks against protocol
version for old features and get rid of {{ProtocolFeatures}} class and it's
interface.
Also, there are code style issues (braces for one-line statements, abbreviation
doesn't used for "context", "version", etc) and some minor comments, I will
review the ticket on github a little bit later.
> ThinClient: Introduce Features for thin clients
> -----------------------------------------------
>
> Key: IGNITE-12853
> URL: https://issues.apache.org/jira/browse/IGNITE-12853
> Project: Ignite
> Issue Type: Bug
> Components: thin client
> Reporter: Igor Sapego
> Assignee: Igor Sapego
> Priority: Major
> Fix For: 2.8.1
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> As we have a lot of different thin clients now, maintained by different
> people, the issues with our backward compatibility mechanism becomes more and
> more prominent.
> Currently, we use protocol versioning as the only approach to provide
> backward compatibility. The main issue of this approach is that we can not
> skip some change in protocol and implement i.e. protocol of version 1.5
> without implementation of 1.4. There are many cases when one may want to do
> so: e.g. when feature provided in 1.4 is not relevant for a specific client,
> or when protocol version 1.5 contains urgent fix or feature which is easy to
> implement, but its blocked by not-so-urgent and hard-to-implement feature
> introduced in 1.4.
> So to fix this issue I propose to introduce another backward compatibility
> mechanism. The idea is to send "supported features" mask by a client to a
> server, which should be answered with the same mask by the server. The
> resulting set of enabled features is acquired with a simple logical "AND"
> operation on these two masks.
> This change has many other positive effects:
> 1. It improves readability and also potentially simplifies debugging.
> 2. It gives users the ability to enable or disable features of thin clients
> on both server and client as they desire.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
