Add support for Live Queries

[Atoo35]

First of all i apologize for the worst code ever.

I am getting this segmentation fault error

In the above image the data print statement is from within ws.read() and I see that I have received the live query response. I think this should have its own type. Being fairly new to go and channels, i created a go routine which keeps extracting data from the connection and i believe this is where i am going wrong I am struggling to understand the reason, but from what I imagine it could be maybe from the ws.read() function since when the server sends the response back to the client, I am reading the response and trying to process it and send it to the response channel created. I think I need to create new set of channels similar to responseChannels and the locks, maybe naming it to notificationChannels but that is later.

[ElecTwix]

I didn't go too far with reviews because of draft PR such as print statements... other than it looking good :) Thanks for your contribution. :+1:

[Atoo35]

@ElecTwix thanks but its not working lol

[ElecTwix]

@ElecTwix thanks but it's not working lol

are you getting seg fault or what?

[Atoo35]

Yes I am getting segmentation fault. That's what's the issue is. I did mention a few points in the description before about why that might be but not sure if that is indeed the reason or not and how to resolve those

[ElecTwix]

Yes I am getting segmentation fault. That's what's the issue is. I did mention a few points in the description before about why that might be but not sure if that is indeed the reason or not and how to resolve those

I will look into it when I had time also in the meantime you could try to debug

[Atoo35]

I think i found the error. So in the gorilla.go file, we have the initialize method which also listens to the socket connection and reads incoming message. This along with the same socket listen code for notifications is problematic. once a response is read, it is "lost", as in both the instances cant read the same message, randomly either the initialize or the LiveNotification code will be invoked causing the other to fail and hence segmentation fault.

So this listening should be done centrally and the response should be handled accordingly. I am not sure how would that be done Attaching the modified initialize function which works fine but not everytime

func (ws *WebSocket) initialize() {
    go func() {
        for {
            select {
            case <-ws.close:
                return
            default:
                var res rpc.RPCResponse
                err := ws.read(&res)
                if err != nil {
                    ws.logger.Logger.Err(err)
                    ws.logger.LogChannel <- err.Error()
                    continue
                }
                var responseChan chan rpc.RPCResponse
                var ok bool
                if res.ID != nil && res.ID != "" {
                    responseChan, ok = ws.getResponseChannel(fmt.Sprintf("%v", res.ID))
                    if !ok {
                        err = errors.New("ResponseChannel is not ok")
                        ws.logger.Logger.Err(err)
                        ws.logger.LogChannel <- err.Error()
                        continue
                    }
                    responseChan <- res
                    close(responseChan)
                } else {
                    fmt.Println("res", res.Result.(map[string]interface{})["id"])
                    responseChan, ok = ws.getResponseChannel(fmt.Sprintf("%v", res.Result.(map[string]interface{})["id"]))
                    if !ok {
                        err = errors.New("ResponseChannel is not ok")
                        ws.logger.Logger.Err(err)
                        ws.logger.LogChannel <- err.Error()
                        continue
                    }
                    responseChan <- res
                    // close(responseChan)
                }

            }
        }
    }()
}

[phughk]

Hey thanks for this! Yes, the stack trace points at the line where nil isn't handled. Either handle or return an error there. It would be a separate PR and I would like a test. I haven't looked into it, but a different design may be viable as a solution as well: where nil isn't possible, for example.

[Atoo35]

@phughk i have pushed some new code. The local test worked fine for the TestLive testcase but apparently something is missing. I am not sure what the issue is. Could you please guide me on the latest code?

If I run all the tests one by on individually, they run fine. but when I run all of them together, they run fine till the TestModify function and then just hangs

[Atoo35]

Can someone please take a look at this? I am out of ideas now 🤣

[Atoo35]

Guys lets get this closed soon please?

[ElecTwix]

Guys let's get this closed soon please?

the last time I talked with @phughk he said the data structure can be changed for live queries when beta 10, maybe we should for beta 10 to see those changes and implement them afterward and wait for that to happen?

[Atoo35]

ahh i seee ok ok

[phughk]

Heya both 👋

We did intend to change the message format. We probably won't. It won't be a big deal if we do, as the mechanism will be essentially the same. So it is safe to program against the current format.

The stack trace can be traced to the file gorilla.go line 273. I haven't looked, but I am guessing that is something uninitialised or defaulting to nil. Have a look at the cause inside the gorilla implementation.

I recommend merging with the main branch as a significant change landed that moves files around (and there are merge conflicts now). This should be done before debugging imo.

[ElecTwix]

The stack trace can be traced to the file gorilla.go line 273. I haven't looked, but I am guessing that is something uninitialized or defaulting to nil. Have a look at the cause inside the gorilla implementation.

I looked into it before it was error caused because it is not killed and the RPC we made will also cause the live query to send back thing so we waited for 1 but we got 2 and that will block other RPC responds because go wait for channel to get that data, I was looking into killing live query after it but I was couldn't manage to do it I will look into it right now maybe I could manage to find, thanks for clarification also

[Atoo35]

just merged the main branch

[ElecTwix]

Hey @Atoo35 I created PR for the loop fix https://github.com/Atoo35/surrealdb.go/pull/1 It is working nightly. Also, @phughk thanks for this https://github.com/surrealdb/surrealdb/pull/2402 it helped a lot

[Atoo35]

@ElecTwix thanks for the help but i swear to god I tried this. why didn't it work lol. lemme try this again xD

[Atoo35]

@ElecTwix you are a saviour, i tried the kill but with an older nightly where it errored out. This is good to go now!!

[timpratim]

Hi @Atoo35, I have added some review comments. The PR is good to be merged once the comments are resolved.

[Atoo35]

Hi @timpratim , thanks for reviewing the code. However, I do see any comments, has github changed the way I need to check for comments or something ?

[timpratim]

They should be visible now. Thanks @Atoo35 .

[Atoo35]

yes i can see them now. Will resolve asap. Thanks!

[ElecTwix]

I left some comments on some of review's comments. I think we need the clear them out before implementing.

[Atoo35]

Thanks @phughk , although this was something I very intially thought needs to be done but I guess we decided in between not to do that if I am not wrong. Anyway thanks for this and sorry for the delay

[timpratim]

Hey @ElecTwix, @Atoo35, we added some fixes to the PR today. Do add your review comments regarding them. I will resolve the previous comments after your review comments.

[phughk]

@Atoo35 yes, must have misunderstood earlier if you were suggesting that already. Thanks for handling the fixes. Looks like i introduced some linting errors. Would it be possible to fix those by you? Or I can open a PR to do that as well.

[Atoo35]

Np @phughk. I will fix the linting issues after work today since I don't have my laptop with me

[phughk]

Amazing thank you @Atoo35 and @ElecTwix !

[phughk]

I have made this an accepted pr for Hacktoberfest @Atoo35 :) Not sure if you are participating

[Atoo35]

Ooohhh cool... Yeah it's embarrassing but I forgot about hacktoberfest haha... Will fix the other PR hopefully tonight post work

[phughk]

Yeah no rush :)