Re: [Tarantool-patches] [PATCH 4/5] vclock: ignore 0th component in comparisons.

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the patch!

See 2 comments below.

On 15/12/2019 21:58, sergepetrenko wrote:
> 0th vclock component will be used to count replica-local rows of an
> anonymous replica. These rows won't be replicated and different
> instances will have different values in vclock[0]. So ignore 0th
> component in comparisons.

1. Lets add 'Part of #3186'.

> ---
>  src/box/vclock.h    | 7 +++++++
>  test/unit/vclock.cc | 8 ++++----
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/src/box/vclock.h b/src/box/vclock.h
> index b5eddcf8b..5dddbd173 100644
> --- a/src/box/vclock.h
> +++ b/src/box/vclock.h
> @@ -281,6 +281,13 @@ vclock_compare(const struct vclock *a, const struct vclock *b)
>  
>  	for (size_t replica_id = bit_iterator_next(&it); replica_id < VCLOCK_MAX;
>  	     replica_id = bit_iterator_next(&it)) {
> +		/*
> +		 * Ignore 0-th component in comparisons.
> +		 * It is empty for normal replicas and should
> +		 * be ignored for anonymous ones.
> +		 */
> +		if (replica_id == 0)
> +			continue;
>  
>  		int64_t lsn_a = vclock_get(a, replica_id);
>  		int64_t lsn_b = vclock_get(b, replica_id);

2. Consider this refactoring in order not to compare replica_id
with 0 on each iteration:

================================================================================

diff --git a/src/box/vclock.h b/src/box/vclock.h
index 5dddbd173..fc6aeb724 100644
--- a/src/box/vclock.h
+++ b/src/box/vclock.h
@@ -279,16 +279,15 @@ vclock_compare(const struct vclock *a, const struct vclock *b)
 	struct bit_iterator it;
 	bit_iterator_init(&it, &map, sizeof(map), true);
 
-	for (size_t replica_id = bit_iterator_next(&it); replica_id < VCLOCK_MAX;
-	     replica_id = bit_iterator_next(&it)) {
-		/*
-		 * Ignore 0-th component in comparisons.
-		 * It is empty for normal replicas and should
-		 * be ignored for anonymous ones.
-		 */
-		if (replica_id == 0)
-			continue;
-
+	size_t replica_id = bit_iterator_next(&it);
+	/*
+	 * Ignore 0-th component in comparisons. It is empty for
+	 * normal replicas and should be ignored for anonymous
+	 * ones.
+	 */
+	if (replica_id == 0)
+		replica_id = bit_iterator_next(&it);
+	for (; replica_id < VCLOCK_MAX; replica_id = bit_iterator_next(&it)) {
 		int64_t lsn_a = vclock_get(a, replica_id);
 		int64_t lsn_b = vclock_get(b, replica_id);
 		le = le && lsn_a <= lsn_b;

================================================================================