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

db-console: allow inflight request replacement for reqs managed by KeyedCacheDataReducer #99134

Closed
THardy98 opened this issue Mar 21, 2023 · 0 comments
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@THardy98
Copy link

THardy98 commented Mar 21, 2023

This issue originates from: #99095

A description of the problem and possible solutions mentioned by @xinhaoz:


So the problem is that we always compare the request time of the current in flight req with the one managed by the CachedDataReducer, and since there's one CachcedDataReducer issuing all reqs in the Keyed one, we always rewrite the pending request time for each new request. So multiple reqs managed by one KeyedCacheReducer will all use the same pendingRequestStarted time in the promise resolver callbacks.

There's a couple of different things we could do as alternatives to the fix here:

  1. In a similar way to address the issue as this PR is currently doing, just add the gate to when we set the pending request time. e.g. (if allowReplacementReqs, then do all the pendingReq setup before we dispatch the request data action / call the api).
  2. An approach that would allow the KeyedCacheDataReducer to allow replacement reqs if we ever desire that (which I assume we might) -- it's a bit more involved, but we essentially just move the fields for comparison of whether or not the request being resolved in the promise is the most recently dispatched req to the state. There's already requestedAt in the state. We can probably use that in place of pendingReqStarted. This approach might be bit finnicky to get to a nice state because the requestedAt is currently set in the reducer, and we dispatch the REQUEST action in the refresh method meaning we can't wait to get the requestedAt synchronously... To circumvent that we can probably provide the requestedAt time when calling requestData().

@THardy98 THardy98 added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-observability labels Mar 21, 2023
@xinhaoz xinhaoz changed the title db-console: improve inflight request replacement db-console: allow inflight request replacement for reqs managed by KeyedCacheDataReducer Mar 21, 2023
@maryliag maryliag closed this as completed Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Development

No branches or pull requests

2 participants