Skip to content

Conversation

@dvoytenko
Copy link
Contributor

This implementation is completely symmetric to the client side. The new article-server.amp.html is the same as article-client.amp.html with the exception of META AMP-Access tag where it declares type=server.

The implementation is fairly complete vis-a-vis client side. The only two pieces missing are autologin and optimistic execution.

Pingback is currently missing in both client and server side and will be prototyped next.

Notes on review: changes to "cache/proxy.js", "cache/server.js", "cache/util.js" and "cache/accessdb.js" are just cruft. The most important changes are in "cache/ampproxy.js" and "cache/client/amp-access.js".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're such an evil maniac :)

Is there a chance of server skew between the 2 requests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Definitely a chance. I called it out somewhere above. We have couple of options: force publishers to specify IDs (and hope they do it right) or build more stable IDs ourselves, e.g. by using path information. Simple transient 1-inf number is definitely not a great idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yet another option: make cache more sticky. E.g. we ensure that we keep this document in cache for, e.g. 15 minutes, with the version#, even if new version is available. Then when server access request arrives, it will contain version# and as much as possible we will try to resolve it against that version.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The right way to do this is to return the version of the doc on the first response and then do a "read at X" on the follow up.

But that really only makes sense if the SCS bigtable can do that efficiently.

@cramforce
Copy link
Member

LGTM

dvoytenko added a commit that referenced this pull request Dec 15, 2015
Server-side access implementation
@dvoytenko dvoytenko merged commit 4558b1d into ampproject:master Dec 15, 2015
@dvoytenko dvoytenko deleted the client6 branch December 15, 2015 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants