ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexey Goncharuk <alexey.goncha...@gmail.com>
Subject Re: Confusing/wrong topology version in GridCacheVersion
Date Mon, 10 Aug 2015 21:34:29 GMT
Denis,

Since I think this is a valuable part of the ticket design, I copied this
email to the ticket comments and responded there, please take a look.

2015-08-10 3:19 GMT-07:00 Denis Magda <dmagda@gridgain.com>:

> Igniters,
>
> While working on IGNITE-946 (
> https://issues.apache.org/jira/browse/IGNITE-946) I faced with a strange
> part of the code.
>
> GridCacheVersion class contains a getter that returns topology version.
> But this is not a plain cluster's topology version we know that is
> increased by a coordinator each time the topology changes.
> It's, according to GridCacheVersion documentation, /"topology version
> *plus number of seconds* from the start time of the first grid node".
> /The documentation is not wrong, just have a look at
> GridCacheVersionManager.next() implementation.
>
> IMHO, this looks like a mess.
>
> What is the reason to have such so called "topologyVersion"?
>
> If there is a reason for that why this sum is called "topologyVersion"?
> It's not just confusing but rather will lead to the bugs in the code.
> Moreover, this is seems to be a bug caused by improper naming:
>
> if (addTime) {
>     if (gridStartTime ==0)
>         gridStartTime =cctx.kernalContext().discovery().gridStartTime();
>
>     topVer += (gridStartTime -TOP_VER_BASE_TIME) /1000;
> }
>
> long globalTime =cctx.kernalContext().clockSync().adjustedTime(topVer);
>
>
> In the code above topVer+time is passed to adjustedTime() method, but
> plain topVer should have been as I guess (GridCacheVersionManager.next()).
>
> --
> Denis
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message