dice icon indicating copy to clipboard operation
dice copied to clipboard

Add integration test for testing WHERE clause in QWATCH.

Open JyotinderSingh opened this issue 1 year ago • 16 comments

WHERE clause support was added to DSQL in #187. However, we don't have any integration test cases validating its behavior.

Requirements

  • Add test cases that validate E2E flow for QWATCH queries using the WHERE clause.
  • See tests/qwatch_test.go for reference.
  • benchmark the code and measure the time taken and memory allocs using benchmem and try to keep them to the bare minimum.
  • Add integration test results as a screenshot in the PR description (since we don't run integration tests on the CI for now)

JyotinderSingh avatar Jul 30 '24 12:07 JyotinderSingh

Hi @JyotinderSingh I would like to pick this.

rsdel2007 avatar Jul 30 '24 12:07 rsdel2007

Hi @JyotinderSingh I would like to pick this.

Assigned

JyotinderSingh avatar Jul 30 '24 12:07 JyotinderSingh

Issue is unassigned now. Feel free to pick this up.

JyotinderSingh avatar Aug 17 '24 09:08 JyotinderSingh

Hi @JyotinderSingh I would like to pick this.

deep-adeshraa avatar Aug 17 '24 15:08 deep-adeshraa

Hi @JyotinderSingh I would like to pick this.

Assigned.

JyotinderSingh avatar Aug 17 '24 15:08 JyotinderSingh

@JyotinderSingh is there any way to overcome comparison issue between string and integer. I am setting up integer in SDK test case but while comparison in where condition, it is giving me error that can't compare string and integer.

deep-adeshraa avatar Aug 21 '24 23:08 deep-adeshraa

@JyotinderSingh is there any way to overcome comparison issue between string and integer. I am setting up integer in SDK test case but while comparison in where condition, it is giving me error that can't compare string and integer.

The issue was fixed recently, please pull the latest master

JyotinderSingh avatar Aug 22 '24 01:08 JyotinderSingh

@deep-adeshraa Thanks a lot for picking this up. Do you have any updates on it? If you have any blockers please discuss this over the discord.

AshwinKul28 avatar Aug 25 '24 18:08 AshwinKul28

Hi @AshwinKul28 I have started working on it but having some difficulties in understanding code. I will try to push asap

deep-adeshraa avatar Aug 25 '24 20:08 deep-adeshraa

Hi @JyotinderSingh @AshwinKul28 - I am working on benchmarking the Qwatch, however I am not sure how to benchmark the code. I am checking BenchmarkLargeByteArray2 tests and seems like we run the function for large number of inputs and check the time taken. Could you please give me some suggestions what would be the best way to benchmark Qwatch ?

deep-adeshraa avatar Aug 29 '24 00:08 deep-adeshraa

Hi @JyotinderSingh @AshwinKul28 - I am working on benchmarking the Qwatch, however I am not sure how to benchmark the code. I am checking BenchmarkLargeByteArray2 tests and seems like we run the function for large number of inputs and check the time taken. Could you please give me some suggestions what would be the best way to benchmark Qwatch ?

A good strategy might be to check performance of watch queries with high number of incoming updates to the query keyset.

The watch query will be looking at a rapidly changing keyset, we should see how long it takes for each new query result to get delivered.

Additionally, another benchmark can be to validate query performance with several queries running in parallel. In such a case you could benchmark both query and SET performance (because each SET operation will compete against the query Executor for lock on the dataset in the current implementation)

JyotinderSingh avatar Aug 29 '24 05:08 JyotinderSingh

Hello @deep-adeshraa,

There has been no activity on this issue for the past 5 days. It would be awesome if you keep posting updates to this issue so that we know you are actively working on it.

We are really eager to close this issue at the earliest, hence if we continue to see the inactivity, we will have to reassign the issue to someone else. We are doing this to ensure that the project maintains its momentum and others are not blocked on this work.

Just drop a comment with the current status of the work or share any issues you are facing. We can always chip in to help you out.

Thanks again.

arpitbbhayani avatar Sep 09 '24 09:09 arpitbbhayani

Hello @arpitbbhayani - I have created PR for integration test cases. However for Benchmark JyotiderSingh asked me to wait until some fixes are deployed. The reason was QWATCH command was failing if I fire large amount of concurrent requests.

deep-adeshraa avatar Sep 09 '24 12:09 deep-adeshraa

Hello @deep-adeshraa,

There has been no activity on this issue for the past 5 days. It would be awesome if you keep posting updates to this issue so that we know you are actively working on it.

We are really eager to close this issue at the earliest, hence if we continue to see the inactivity, we will have to reassign the issue to someone else. We are doing this to ensure that the project maintains its momentum and others are not blocked on this work.

Just drop a comment with the current status of the work or share any issues you are facing. We can always chip in to help you out.

Thanks again.

arpitbbhayani avatar Sep 20 '24 18:09 arpitbbhayani

@JyotinderSingh - please assign to me.

psrvere avatar Sep 25 '24 07:09 psrvere

Hello @psrvere,

There has been no activity on this issue for the past 5 days. It would be awesome if you keep posting updates to this issue so that we know you are actively working on it.

We are really eager to close this issue at the earliest, hence if we continue to see the inactivity, we will have to reassign the issue to someone else. We are doing this to ensure that the project maintains its momentum and others are not blocked on this work.

Just drop a comment with the current status of the work or share any issues you are facing. We can always chip in to help you out.

Thanks again.

arpitbbhayani avatar Oct 03 '24 14:10 arpitbbhayani