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

fix(sqllab): race condition when updating cursor position #30154

Conversation

justinpark
Copy link
Member

@justinpark justinpark commented Sep 4, 2024

SUMMARY

#30141 resolved the initial SQLLab page loading issue, but I identified that the same race condition error occurs when the cursor moves.
The problem is that the cursorPosition should only be accessed during onLoad to set the initial cursor position, but an issue arises where the Ace Editor is re-rendered whenever the cursorPosition is changed via useQueryEditor selector. During rerendering, the Ace Editor keeps resetting the cursor position, triggering the cursorUpdate event, which in turn causes onCursorPositionChange to be called repeatedly, leading to an infinite cursor position update loop.
To resolve this, avoided accessing the cursor position in real-time from the unsaved state and limited access to the initial state only, thereby preventing the number of re-renders.

TESTING INSTRUCTIONS

Specs are added

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@dosubot dosubot bot added the sqllab Namespace | Anything related to the SQL Lab label Sep 4, 2024
@michael-s-molina michael-s-molina added v4.0 Label added by the release manager to track PRs to be included in the 4.0 branch v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch labels Sep 4, 2024
@justinpark justinpark merged commit 2097b71 into apache:master Sep 4, 2024
39 of 41 checks passed
sadpandajoe pushed a commit that referenced this pull request Sep 5, 2024
michael-s-molina pushed a commit that referenced this pull request Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M sqllab Namespace | Anything related to the SQL Lab v4.0 Label added by the release manager to track PRs to be included in the 4.0 branch v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants