Title
#users-public
b

Bartosz

09/12/2022, 7:13 PM
Hello again today 🙂 I've found some bug. I'm using nodejs-questdb-client. My script collecting data from some devices and these data are analysed as have to be stored or not. There is something strange. For sure i created extra console.log to be sure if script make one row - unfortunelly when i check data on QuestDB, somethimes i've duplicates - You can see on screenshot. There should be only 3 rows.
8:16 PM
The problem occurs when I load more data into the Buffer before sending it.
8:31 PM
OK, After several dozen minutes, you can see that the data is still duplicated, but with less frequency
Imre

Imre

09/12/2022, 8:52 PM
hi Bartosz, any chance you can share some of your code? What I would like to see is how I could reproduce the issue.
b

Bartosz

09/19/2022, 8:25 PM
@Imre Here is my code... it's work together with Redis... Redis hold actual state of physical devices and when old value is different than new and other stuff return true, then QuestDB get new row
8:26 PM
Maybe @Bolek Ziobrowski @Jaromir Hamala @Eugene have little bit time, to take a look, why i've duplicates in database
8:30 PM
if( address === 'P30v2' ) {
 console.log(devid, address, value, timestamp, tableName)
}
8:30 PM
Line above return one row in console but in database it is sometimes 4-8 similar rows
8:38 PM
and i using one QuestDB connection for collecting data from 2k+ IoT devices
Imre

Imre

09/20/2022, 9:17 AM
hi @Bartosz, let me take a look at this today
Jaromir Hamala

Jaromir Hamala

09/20/2022, 9:35 AM
silly idea: can you try to change
sender.flush();
to
await sender.flush();
?
Imre

Imre

09/20/2022, 9:47 AM
i think that should fix it
Jaromir Hamala

Jaromir Hamala

09/20/2022, 9:48 AM
Hello @Bartosz, I just consulted this with Imre and he confirmed it’s the missing
await
in flushing. The
flush()
methods returns a promise and you need to wait for it to resolve before calling it again. If you call
flush()
twice without waiting for the first flush promise to resolve then you can get dup rows.
Imre

Imre

09/20/2022, 9:49 AM
i will make a change today to prevent this
b

Bartosz

09/20/2022, 9:49 AM
Thank you guys... i'll check this solution
Jaromir Hamala

Jaromir Hamala

09/20/2022, 9:58 AM
please post the outcome, both negative and positive! thanks!
b

Bartosz

09/20/2022, 9:58 AM
Yes of course i'll 😉
11:04 AM
Ok. here is result:
11:04 AM
2022-09-20T12:39:51: YKRLSRGWXB P30v2 348 1663670391 uint16b
2022-09-20T12:41:01: YKRLSRGWXB P30v2 346 1663670461 uint16b
2022-09-20T12:41:26: YKRLSRGWXB P30v2 345 1663670485 uint16b
2022-09-20T12:41:47: YKRLSRGWXB P30v2 344 1663670506 uint16b
2022-09-20T12:43:14: YKRLSRGWXB P30v2 343 1663670593 uint16b
2022-09-20T12:47:25: YKRLSRGWXB P30v2 341 1663670844 uint16b
2022-09-20T12:52:48: YKRLSRGWXB P30v2 340 1663671167 uint16b
2022-09-20T12:56:54: YKRLSRGWXB P30v2 339 1663671414 uint16b
11:04 AM
Each row in console.log mean creating entry in Sender buffor
11:04 AM
i've modyfied my code to
Jaromir Hamala

Jaromir Hamala

09/20/2022, 11:05 AM
in other words: there are still duplicates
b

Bartosz

09/20/2022, 11:05 AM
RedisPublisher.pipeline(memoryQuery).exec(async (error, response) => { (...) await sender.flush(); })
11:06 AM
Unfortunately yes 😞
Jaromir Hamala

Jaromir Hamala

09/20/2022, 11:07 AM
can you share the full code?
11:07 AM
with your latest chances
b

Bartosz

09/20/2022, 11:07 AM
Yes, just a sec
Jaromir Hamala

Jaromir Hamala

09/20/2022, 11:08 AM
I think there is still a chance the
flush()
is called multiple times
b

Bartosz

09/20/2022, 11:12 AM
I'll also make test with puting flush after for while as was before my thread
Jaromir Hamala

Jaromir Hamala

09/20/2022, 11:16 AM
I think there is still a problem with multiple flush: imagine a subscriber receives a new event. this calls
onMessageSetAll()
then it calls
await sender.flush()
and it’s wiating for the flush promise to resolve. this yield the thread. while it’s waiting for the promise to resolve there is a new message received by the
RedisSubscriber
. so it calls
onMessageSetAll()
again and ultimately
sender.flush()
will be called yet again. even before the first flush promise is resolved.
11:18 AM
I think this is very hard to change in your code. we need to change our nodejs client so it won’t produce duplicates when flush is called multiple times before waiting for a promise to resolve. Imre is already working on it.
b

Bartosz

09/20/2022, 11:18 AM
So creating apart buffor per request should resolve problem
11:21 AM
I suspected that there might be another onMessageSetAll call before the buffer underrun, but I can't do it synchronously because I will clog the server in a short time
Imre

Imre

09/20/2022, 11:28 AM
@Bartosz, looks to me that there is another flush() without await in onMessageKeyExpired()
11:28 AM
any chance you can add await there as well?
b

Bartosz

09/20/2022, 11:29 AM
ok, i'll run with this
Imre

Imre

09/20/2022, 11:29 AM
that is still creating promises which you do not wait for
b

Bartosz

09/20/2022, 11:37 AM
@Imre, this not resolve problem. I think is because there coming to much request in same time which work on one Sender Buffor as sugesting also @Jaromir Hamala
11:39 AM
Earlier i was using pure Node net library and there wasn't duplicates, because i was wrote to socket directly
Imre

Imre

09/20/2022, 12:03 PM
checking it now
2:44 PM
hi @Bartosz, did my best and tried to reproduce the problem but could not. i think the client should be fine and if you add await in front of all flush() calls it should work properly. if you still see the duplicates, could you try to create a minimum reproducer, please?
b

Bartosz

09/20/2022, 8:12 PM
@Imre, @Jaromir Hamala i've created reproducer, but i can't reach similar effect like on production code with 2k+ IoT devices
Imre

Imre

09/20/2022, 8:30 PM
@Bartosz, we have discussed this with @Jaromir Hamala and the problem seems to be that flush() calls are overlapping when events are handled. basically you should treat the client the same way as if you were writing to a socket directly. the client gives you the benefit of validating messages before they are sent to the server, it also escapes special characters, makes the protocol easier to use but it still uses the socket API underneath. you said that before you used a socket directly and it worked, maybe you could use the client the same way? i will also think about it if we can do anything in the client.
b

Bartosz

09/20/2022, 8:38 PM
Yes, tomorrow i'll change my code to send again data directly to socket. I'm almost sure, when every call to onMessageSetAll 'll create new instance of Sender, then problem 'll be resolved, because every request have apart buffor, but underneath it's create another problem with too much opened connections to database.
Imre

Imre

09/20/2022, 9:19 PM
i think there is something we can do. if flush() creates a new buffer containing the data to be sent to the server and clears the sender’s main buffer then basically each promise calling socket.write() will have its own buffer with the data belongs to it. was not keen making this change because it is a huge performance penalty for those who does not have overlapping calls to flush(). will make it configurable so you can choose to make copies of the buffer if you need it. i think this is much better than creating new Sender instances. let me give this a go, will update tomorrow.
b

Bartosz

09/21/2022, 6:15 AM
I can't wait @Imre 🙂
Imre

Imre

09/21/2022, 10:51 AM
hi @Bartosz, release 1.0.1 with the new option
b

Bartosz

09/21/2022, 10:52 AM
I'll test this tonight
Imre

Imre

09/21/2022, 10:52 AM
the above link shows an example, how to switch the flag on
b

Bartosz

09/21/2022, 11:45 AM
I see problem was resolved after 5 min test 🙂 Now i take break until tonight, so i'll give you more results in longer test 🙂
11:47 AM
and i don't see difference with perfomance
Imre

Imre

09/21/2022, 12:19 PM
that is very good news
Jaromir Hamala

Jaromir Hamala

09/21/2022, 4:03 PM
excellent, thanks for sharing!
Imre

Imre

09/21/2022, 8:49 PM
Glad it works. Thank you