Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-try executing current buffer on broken connection #40

Closed
elprans opened this issue May 14, 2020 · 6 comments
Closed

Re-try executing current buffer on broken connection #40

elprans opened this issue May 14, 2020 · 6 comments
Assignees
Projects

Comments

@elprans
Copy link
Member

elprans commented May 14, 2020

Currently, when the server goes away, REPL detects that and reconnects, but it doesn't execute the command I issued right away, which is annoying, because the dev cycle requires restarting the server frequently.

@tailhook
Copy link
Contributor

This is a very tough question. In case we know that request has not been sent, we can do that. But in case we sent request and then connection interrupted we must not send the request. Currently, we don't have a way to detect the former. I'll review this after Flush is fixed (since I have to refactor a chunk of networking code at that time).

@1st1
Copy link
Member

1st1 commented May 15, 2020

This is a very tough question.

Yeah, I agree. On top of that, we mustn't re-run queries if the previous connection was in transaction. When you take all limitations & caveats to account it looks like this isn't a great feature (except maybe for development of EdgeDB itself).

So I'm -1 on this.

@vpetrovykh
Copy link
Member

Perhaps we can leverage our knowledge of what state the REPL was in prior to connection break to provide additional information in the error message. Such as explicitly state that the transaction was cancelled if we were in the middle of transaction. In fact, perhaps we may want to indicate whether we're in transaction or not somehow. A visual cue whether there's an active transaction in REPL is helpful when experimenting and you really want to be in a transaction for your stuff.

@1st1
Copy link
Member

1st1 commented May 19, 2020

So I'm -1 on this.

To elaborate, as discussed off-list, we decided to implement this only in dev mode.

@tailhook
Copy link
Contributor

My afterthought: any kind of request retry even unconditionally in devmode requires quite a bit of refactoring in client.rs code because connection is owned by frame up on the stack than the eval loop. So it's unlikely that it can be done for Alpha 3.

Other than that, it's probably good to automagically repeat requests if connection is broken before request when not in transaction and show a hint otherwise.

@vpetrovykh
Copy link
Member

Other than that, it's probably good to automagically repeat requests if connection is broken before request when not in transaction and show a hint otherwise.

Yeah, that would incidentally cover many of our actual dev use-cases: we restart the server between queries, so if a connection is broken we should be able to know this before the query runs.

@1st1 1st1 added this to All tasks (release on: July 13th) in Alpha 4 via automation Jun 1, 2020
@tailhook tailhook moved this from All tasks (release on: July 13th) to In progress in Alpha 4 Jun 1, 2020
tailhook added a commit that referenced this issue Jun 18, 2020
tailhook added a commit that referenced this issue Jun 18, 2020
Alpha 4 automation moved this from In progress to Done Jun 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Alpha 4
  
Done
Development

No branches or pull requests

4 participants