Fix for stale staging tree loading by rtibbles · Pull Request #2884 · learningequality/studio

rtibbles · 2021-01-28T00:47:35Z

Replicates where stale while invalidate behaviour in get - as opposed to the previous stale without invalidate
Redirects to the root staging view when we detect that the root staging id has changed

@bjester interested in your thoughts on the change in the get default behaviour for cache invalidation.

jayoshih · 2021-01-28T16:44:49Z

 return obj;
 }
- return this.requestModel(id);
+ return request;


Could you explain the difference here?

We are now making the request, regardless of whether the data was found in IndexedDB or not, so we serve the stale data in the mean time, but invalidate the stale cache with the request.

I know some of the copy and move commands are fairly rapid-fire, so this could produce a lot of requests. I'm not sure of the impact but combined with the other recent change I made, the net result could not be so great. This doesn't necessarily help that, but what if in the case when we do have a copy locally, we instead put the requestModel call in the next tick?

I think the overall effect of the indirection in requestModel does mean that it gets shunted out of the main thread and picked up later. I could also add the same sort of request sharing that we are using for requestCollection where we maintain the fetch promise for 5 seconds and just pass it back if the same request is made again.

bjester

I selected multiple nodes and did some copying and it seemed okay!

rtibbles · 2021-01-28T00:47:35Z

Replicates where stale while invalidate behaviour in get - as opposed to the previous stale without invalidate
Redirects to the root staging view when we detect that the root staging id has changed

Fixes#2361

@bjester interested in your thoughts on the change in the get default behaviour for cache invalidation.

jayoshih · 2021-01-28T16:44:49Z

 return obj;
 }
- return this.requestModel(id);
+ return request;


Could you explain the difference here?

We are now making the request, regardless of whether the data was found in IndexedDB or not, so we serve the stale data in the mean time, but invalidate the stale cache with the request.

I know some of the copy and move commands are fairly rapid-fire, so this could produce a lot of requests. I'm not sure of the impact but combined with the other recent change I made, the net result could not be so great. This doesn't necessarily help that, but what if in the case when we do have a copy locally, we instead put the requestModel call in the next tick?

I think the overall effect of the indirection in requestModel does mean that it gets shunted out of the main thread and picked up later. I could also add the same sort of request sharing that we are using for requestCollection where we maintain the fetch promise for 5 seconds and just pass it back if the same request is made again.

bjester

I selected multiple nodes and did some copying and it seemed okay!

rtibbles · 2021-01-28T00:47:35Z

Replicates where stale while invalidate behaviour in get - as opposed to the previous stale without invalidate
Redirects to the root staging view when we detect that the root staging id has changed

Fixes#2361

@bjester interested in your thoughts on the change in the get default behaviour for cache invalidation.

jayoshih · 2021-01-28T16:44:49Z

 return obj;
 }
- return this.requestModel(id);
+ return request;


Could you explain the difference here?

We are now making the request, regardless of whether the data was found in IndexedDB or not, so we serve the stale data in the mean time, but invalidate the stale cache with the request.

I know some of the copy and move commands are fairly rapid-fire, so this could produce a lot of requests. I'm not sure of the impact but combined with the other recent change I made, the net result could not be so great. This doesn't necessarily help that, but what if in the case when we do have a copy locally, we instead put the requestModel call in the next tick?

I think the overall effect of the indirection in requestModel does mean that it gets shunted out of the main thread and picked up later. I could also add the same sort of request sharing that we are using for requestCollection where we maintain the fetch promise for 5 seconds and just pass it back if the same request is made again.

bjester

I selected multiple nodes and did some copying and it seemed okay!

rtibbles · 2021-01-28T00:47:35Z

Replicates where stale while invalidate behaviour in get - as opposed to the previous stale without invalidate
Redirects to the root staging view when we detect that the root staging id has changed

Fixes#2361

@bjester interested in your thoughts on the change in the get default behaviour for cache invalidation.

jayoshih · 2021-01-28T16:44:49Z

 return obj;
 }
- return this.requestModel(id);
+ return request;


Could you explain the difference here?

We are now making the request, regardless of whether the data was found in IndexedDB or not, so we serve the stale data in the mean time, but invalidate the stale cache with the request.

I know some of the copy and move commands are fairly rapid-fire, so this could produce a lot of requests. I'm not sure of the impact but combined with the other recent change I made, the net result could not be so great. This doesn't necessarily help that, but what if in the case when we do have a copy locally, we instead put the requestModel call in the next tick?

I think the overall effect of the indirection in requestModel does mean that it gets shunted out of the main thread and picked up later. I could also add the same sort of request sharing that we are using for requestCollection where we maintain the fetch promise for 5 seconds and just pass it back if the same request is made again.

bjester

I selected multiple nodes and did some copying and it seemed okay!

rtibbles · 2021-01-28T00:47:35Z

Replicates where stale while invalidate behaviour in get - as opposed to the previous stale without invalidate
Redirects to the root staging view when we detect that the root staging id has changed

Fixes#2361

@bjester interested in your thoughts on the change in the get default behaviour for cache invalidation.

jayoshih · 2021-01-28T16:44:49Z

 return obj;
 }
- return this.requestModel(id);
+ return request;


Could you explain the difference here?

We are now making the request, regardless of whether the data was found in IndexedDB or not, so we serve the stale data in the mean time, but invalidate the stale cache with the request.

I know some of the copy and move commands are fairly rapid-fire, so this could produce a lot of requests. I'm not sure of the impact but combined with the other recent change I made, the net result could not be so great. This doesn't necessarily help that, but what if in the case when we do have a copy locally, we instead put the requestModel call in the next tick?

I think the overall effect of the indirection in requestModel does mean that it gets shunted out of the main thread and picked up later. I could also add the same sort of request sharing that we are using for requestCollection where we maintain the fetch promise for 5 seconds and just pass it back if the same request is made again.

bjester

I selected multiple nodes and did some copying and it seemed okay!

rtibbles · 2021-01-28T00:47:35Z

Replicates where stale while invalidate behaviour in get - as opposed to the previous stale without invalidate
Redirects to the root staging view when we detect that the root staging id has changed

Fixes#2361

@bjester interested in your thoughts on the change in the get default behaviour for cache invalidation.

jayoshih · 2021-01-28T16:44:49Z

 return obj;
 }
- return this.requestModel(id);
+ return request;


Could you explain the difference here?

We are now making the request, regardless of whether the data was found in IndexedDB or not, so we serve the stale data in the mean time, but invalidate the stale cache with the request.

I know some of the copy and move commands are fairly rapid-fire, so this could produce a lot of requests. I'm not sure of the impact but combined with the other recent change I made, the net result could not be so great. This doesn't necessarily help that, but what if in the case when we do have a copy locally, we instead put the requestModel call in the next tick?

I think the overall effect of the indirection in requestModel does mean that it gets shunted out of the main thread and picked up later. I could also add the same sort of request sharing that we are using for requestCollection where we maintain the fetch promise for 5 seconds and just pass it back if the same request is made again.

bjester

I selected multiple nodes and did some copying and it seemed okay!

rtibbles · 2021-01-28T00:47:35Z

Replicates where stale while invalidate behaviour in get - as opposed to the previous stale without invalidate
Redirects to the root staging view when we detect that the root staging id has changed

Fixes#2361

@bjester interested in your thoughts on the change in the get default behaviour for cache invalidation.

jayoshih · 2021-01-28T16:44:49Z

 return obj;
 }
- return this.requestModel(id);
+ return request;


Could you explain the difference here?

We are now making the request, regardless of whether the data was found in IndexedDB or not, so we serve the stale data in the mean time, but invalidate the stale cache with the request.

I know some of the copy and move commands are fairly rapid-fire, so this could produce a lot of requests. I'm not sure of the impact but combined with the other recent change I made, the net result could not be so great. This doesn't necessarily help that, but what if in the case when we do have a copy locally, we instead put the requestModel call in the next tick?

I think the overall effect of the indirection in requestModel does mean that it gets shunted out of the main thread and picked up later. I could also add the same sort of request sharing that we are using for requestCollection where we maintain the fetch promise for 5 seconds and just pass it back if the same request is made again.

bjester

I selected multiple nodes and did some copying and it seemed okay!

rtibbles · 2021-01-28T00:47:35Z

Replicates where stale while invalidate behaviour in get - as opposed to the previous stale without invalidate
Redirects to the root staging view when we detect that the root staging id has changed

Fixes#2361

@bjester interested in your thoughts on the change in the get default behaviour for cache invalidation.

jayoshih · 2021-01-28T16:44:49Z

 return obj;
 }
- return this.requestModel(id);
+ return request;


Could you explain the difference here?

We are now making the request, regardless of whether the data was found in IndexedDB or not, so we serve the stale data in the mean time, but invalidate the stale cache with the request.

I know some of the copy and move commands are fairly rapid-fire, so this could produce a lot of requests. I'm not sure of the impact but combined with the other recent change I made, the net result could not be so great. This doesn't necessarily help that, but what if in the case when we do have a copy locally, we instead put the requestModel call in the next tick?

I think the overall effect of the indirection in requestModel does mean that it gets shunted out of the main thread and picked up later. I could also add the same sort of request sharing that we are using for requestCollection where we maintain the fetch promise for 5 seconds and just pass it back if the same request is made again.

bjester

I selected multiple nodes and did some copying and it seemed okay!

rtibbles added 2 commits January 27, 2021 14:20

Do stale while invalidate for gets.
ebbcd48

Redirect to the root staging view if the staging root id changes.
fea5dfc

rtibbles added this to the Post Release Stabilization milestone Jan 28, 2021

rtibbles requested review from bjester and jayoshih January 28, 2021 00:47

jayoshih approved these changes Jan 28, 2021
View reviewed changes

jayoshih reviewed Jan 28, 2021
View reviewed changes

rtibbles mentioned this pull request Jan 28, 2021
Navigate to new staging tree if staging tree has changed #2883
Closed

bjester approved these changes Jan 29, 2021
View reviewed changes

rtibbles merged commit 081d96a into learningequality:hotfixesJan 29, 2021

rtibbles deleted the stale_while_invalidate_get branch January 29, 2021 20:47

rtibbles added 2 commits January 27, 2021 14:20

Do stale while invalidate for gets.
ebbcd48

Redirect to the root staging view if the staging root id changes.
fea5dfc

rtibbles added this to the Post Release Stabilization milestone Jan 28, 2021

rtibbles requested review from bjester and jayoshih January 28, 2021 00:47

jayoshih approved these changes Jan 28, 2021
View reviewed changes

jayoshih reviewed Jan 28, 2021
View reviewed changes

rtibbles mentioned this pull request Jan 28, 2021
Navigate to new staging tree if staging tree has changed #2883
Closed

bjester approved these changes Jan 29, 2021
View reviewed changes

rtibbles merged commit 081d96a into learningequality:hotfixesJan 29, 2021

rtibbles deleted the stale_while_invalidate_get branch January 29, 2021 20:47

rtibbles added 2 commits January 27, 2021 14:20

Do stale while invalidate for gets.
ebbcd48

Redirect to the root staging view if the staging root id changes.
fea5dfc

rtibbles added this to the Post Release Stabilization milestone Jan 28, 2021

rtibbles requested review from bjester and jayoshih January 28, 2021 00:47

jayoshih approved these changes Jan 28, 2021
View reviewed changes

jayoshih reviewed Jan 28, 2021
View reviewed changes

rtibbles mentioned this pull request Jan 28, 2021
Navigate to new staging tree if staging tree has changed #2883
Closed

bjester approved these changes Jan 29, 2021
View reviewed changes

rtibbles merged commit 081d96a into learningequality:hotfixesJan 29, 2021

rtibbles deleted the stale_while_invalidate_get branch January 29, 2021 20:47

rtibbles added 2 commits January 27, 2021 14:20

Do stale while invalidate for gets.
ebbcd48

Redirect to the root staging view if the staging root id changes.
fea5dfc

rtibbles added this to the Post Release Stabilization milestone Jan 28, 2021

rtibbles requested review from bjester and jayoshih January 28, 2021 00:47

jayoshih approved these changes Jan 28, 2021
View reviewed changes

jayoshih reviewed Jan 28, 2021
View reviewed changes

rtibbles mentioned this pull request Jan 28, 2021
Navigate to new staging tree if staging tree has changed #2883
Closed

bjester approved these changes Jan 29, 2021
View reviewed changes

rtibbles merged commit 081d96a into learningequality:hotfixesJan 29, 2021

rtibbles deleted the stale_while_invalidate_get branch January 29, 2021 20:47

rtibbles added 2 commits January 27, 2021 14:20

Do stale while invalidate for gets.
ebbcd48

Redirect to the root staging view if the staging root id changes.
fea5dfc

rtibbles added this to the Post Release Stabilization milestone Jan 28, 2021

rtibbles requested review from bjester and jayoshih January 28, 2021 00:47

jayoshih approved these changes Jan 28, 2021
View reviewed changes

jayoshih reviewed Jan 28, 2021
View reviewed changes

rtibbles mentioned this pull request Jan 28, 2021
Navigate to new staging tree if staging tree has changed #2883
Closed

bjester approved these changes Jan 29, 2021
View reviewed changes

rtibbles merged commit 081d96a into learningequality:hotfixesJan 29, 2021

rtibbles deleted the stale_while_invalidate_get branch January 29, 2021 20:47

rtibbles added 2 commits January 27, 2021 14:20

Do stale while invalidate for gets.
ebbcd48

Redirect to the root staging view if the staging root id changes.
fea5dfc

rtibbles added this to the Post Release Stabilization milestone Jan 28, 2021

rtibbles requested review from bjester and jayoshih January 28, 2021 00:47

jayoshih approved these changes Jan 28, 2021
View reviewed changes

jayoshih reviewed Jan 28, 2021
View reviewed changes

rtibbles mentioned this pull request Jan 28, 2021
Navigate to new staging tree if staging tree has changed #2883
Closed

bjester approved these changes Jan 29, 2021
View reviewed changes

rtibbles merged commit 081d96a into learningequality:hotfixesJan 29, 2021

rtibbles deleted the stale_while_invalidate_get branch January 29, 2021 20:47

rtibbles added 2 commits January 27, 2021 14:20

Do stale while invalidate for gets.
ebbcd48

Redirect to the root staging view if the staging root id changes.
fea5dfc

rtibbles added this to the Post Release Stabilization milestone Jan 28, 2021

rtibbles requested review from bjester and jayoshih January 28, 2021 00:47

jayoshih approved these changes Jan 28, 2021
View reviewed changes

jayoshih reviewed Jan 28, 2021
View reviewed changes

rtibbles mentioned this pull request Jan 28, 2021
Navigate to new staging tree if staging tree has changed #2883
Closed

bjester approved these changes Jan 29, 2021
View reviewed changes

rtibbles merged commit 081d96a into learningequality:hotfixesJan 29, 2021

rtibbles deleted the stale_while_invalidate_get branch January 29, 2021 20:47

rtibbles added 2 commits January 27, 2021 14:20

Do stale while invalidate for gets.
ebbcd48

Redirect to the root staging view if the staging root id changes.
fea5dfc

rtibbles added this to the Post Release Stabilization milestone Jan 28, 2021

rtibbles requested review from bjester and jayoshih January 28, 2021 00:47

jayoshih approved these changes Jan 28, 2021
View reviewed changes

jayoshih reviewed Jan 28, 2021
View reviewed changes

rtibbles mentioned this pull request Jan 28, 2021
Navigate to new staging tree if staging tree has changed #2883
Closed

bjester approved these changes Jan 29, 2021
View reviewed changes

rtibbles merged commit 081d96a into learningequality:hotfixesJan 29, 2021

rtibbles deleted the stale_while_invalidate_get branch January 29, 2021 20:47

Uh oh!

Uh oh!

Conversation

rtibbles commented Jan 28, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bjester left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

rtibbles commented Jan 28, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bjester left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

rtibbles commented Jan 28, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bjester left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

rtibbles commented Jan 28, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

rtibbles commented Jan 28, 2021•
edited
Loading

rtibbles commented Jan 28, 2021•
edited
Loading

rtibbles commented Jan 28, 2021•
edited
Loading

rtibbles commented Jan 28, 2021•
edited
Loading

rtibbles commented Jan 28, 2021•
edited
Loading