what is outgoing:undefined in redis #98
Comments
b-sirius-a1v
changed the title
|
Does downgrading to 8.0.1 fixes the issue? |
|
Thank you very much!!! I have some questions
|
|
@seriousme I think we may have a regression here, maybe introduced by the sanity check on keys? |
|
It will be some sort of regression if it works on 8.0.1, however @b-sirius-a1v if you try the following: node -e "console.log(encodeURIComponent('blah'))"Where 'blah' is replaced by the real name of C1, do you then get Kind regards, |
|
I just checked, I can reproduce the problem e.g.: Running However I also see many I will check later to see if I can find which test(s) cause the Kind regards, |
|
Ok, there were 7 occurences of The first 3: are caused by: needs to be added to the The next 3: are caused by: needs to be added to the The last one is: And that is caused by: aedes-persistence-redis/test.js Line 126 in 10a9fc8 Where With these fixes in I don't see any So the One thing that comes to mind is that Kind regards, |
|
Small update: I just tried to replicate the issue on The subscriber now sees The subscriber only sees So the problem seems to be older than 9.0.0. Kind regards, |
|
Ok, my bad :-( This shows Now we stop everything, including Redis to clean the cache ! The Redis Monitor log shows for 9.0.0: The Redis Monitor log shows for 8.0.1: The major difference being in 9.0.0: instead of in 8.0.1: Now earlier tests have already shown that Kind regards, |
|
OK, I did some more digging and found the issue :-) The issue is not with the Unfortunately in: aedes-persistence-redis/persistence.js Line 232 in 10a9fc8 we should have put: that._db.hgetallBuffer(clientSubKey(clientId), function clientHash (err, hash) {since we also do: aedes-persistence-redis/persistence.js Line 128 in 10a9fc8 An similarly we should use aedes-persistence-redis/persistence.js Line 565 in 10a9fc8 Fixing those 2 results in correct loading of stored subscriptions on restart. I will add a PR for this. Kind regards, |
|
Btw: having some method of type checking would have helped in avoiding the need for all 3 PR's ;-) |
|
One more thing: seems to be a bug in Aedes itself, caused by inconsistencies in the interface :-( The sequence comes from the delWill function. What seems to be a bit odd is that Looking at Aedes there are 4 locations that call
My suggestion would be to explicitly pass the brokerId tot Kind regards, |
This makes sense as in a cluster env dellWill should delete any existing will message with that clientId without considering brokerId that could be a different one. If you see mongodb persistence it uses only the client id to identify a will message. This is a bug that should be fixed on aedes persistence IMO, the will key should not contain the broker id |
|
Edit: I see that NPM is still on 9.0.0, so following may be somewhat irrelevant. I think this is still broken. relevant package dependencies in subscriptionsByTopic, I'm logging as follows: which yields - Which of course inserts undefined
|
|
I'm not sure which is the correct place to post, but it would be good to update dependencies to require the correct ones, as well as release redis persistence to npm, otherwise the standard npm install aedes-persistence-redis is going to yield these results. I think it requires "aedes-persistence": "^9.1.2", |
|
It already uses aedes-persistence 9.1.2, it uses aedes-cached-persistence 9 that uses it: https://github.com/moscajs/aedes-cached-persistence/blob/master/package.json |
System Information
I'm using a translator
Describe the bug
outgoing:undefinedis generated when broker is restarted with client of qos=1When a client goes offline and back online, the broker no longer provides a buffer for the client
To Reproduce
Steps to reproduce the behavior:
qos=1qos=1outgoing:undefinedhas been created in redisqos=1The text was updated successfully, but these errors were encountered: