hazelcast / hazelcast-nodejs-client

Hazelcast Node.js Client
https://hazelcast.com/clients/node-js/
Apache License 2.0
150 stars 58 forks source link

Fix backward compatibility test v5.2.0 #1435

Closed harunalpak closed 1 year ago

harunalpak commented 1 year ago

Related to https://github.com/hazelcast/hazelcast-nodejs-client/pull/1418 this PR we added a new Codec named "SetUUIDCodec".

And this is used in SchemaService.put method.

replicateSchemaInCluster(schema: Schema): Promise<boolean> {
        const clientMessage = ClientSendSchemaCodec.encodeRequest(schema);
        return this.retryMaxPutRetryCount(clientMessage, 0);
    }

private retryMaxPutRetryCount(clientMessage: ClientMessage, currentRetryCount: number) : Promise<boolean> {
    if (currentRetryCount === this.maxPutRetryCount) {
        return Promise.resolve(false);
    }
    const invocationService = this.getInvocationService(); 
    const invocation = new Invocation(invocationService, clientMessage);
    return invocationService.invoke(invocation).then((response) => {
        const replicatedMemberUuids = UuidUtil.convertUUIDSetToStringSet(ClientSendSchemaCodec.**decodeResponse**(response));
        const members = this.getClusterService().getMembers();
        for (const member of members) {
            if (!replicatedMemberUuids.has(member.uuid.toString())) {
                // There is a member in our member list that the schema
                // is not known to be replicated yet. We should retry
                // sending it in a random member.
                return delayedPromise(this.retryPauseMillis).then(() => {
                    // correlation id will be set when the invoke method is
                    // called above
                    clientMessage = clientMessage.copyMessageWithSharedNonInitialFrames();

                    return this.retryMaxPutRetryCount(clientMessage, ++currentRetryCount);
                })
            }
        }

        // All members in our member list all known to have the schema
        return Promise.resolve(true);
    })

}

And I check that it is also added to server version 5.2.0. (https://github.com/hazelcast/hazelcast/pull/22197)

IMO 'CompactTest' should run greater than 5.2.0 server version.

srknzl commented 1 year ago

@mdumandag This means that compact won't work with a 5.1.z server. So it is a breaking change. Is this expected?

codecov[bot] commented 1 year ago

Codecov Report

Merging #1435 (685323c) into master (67ea00f) will increase coverage by 0.02%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1435      +/-   ##
==========================================
+ Coverage   93.17%   93.19%   +0.02%     
==========================================
  Files         466      466              
  Lines       16616    16616              
  Branches     1348     1348              
==========================================
+ Hits        15482    15486       +4     
+ Misses        832      827       -5     
- Partials      302      303       +1     
Impacted Files Coverage Δ
src/util/Util.ts 86.30% <0.00%> (-0.69%) :arrow_down:
src/invocation/InvocationService.ts 94.87% <0.00%> (ø)
src/network/ConnectionManager.ts 79.81% <0.00%> (+1.16%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

srknzl commented 1 year ago

approving, discussed via slack. It is expected that we break compatiblity