TRS WD 2021-05-19 Part 1 review comments

[berezovskyi]

Part 1

• [x] 1. Should the §1.1 Terminology be non-normative?
• [x] 2. Should the §1.2 RFC Terms be non-normative? It even makes statements on what is normative in the spec.
• [x] 3. /§1.2/ is "Fig. 1" an appropriate caption label for code example?
• [ ] 4. /§3/ reword "Note: Discovery is a convenience" and the following para. They also sound quite non-normative. Nick: this is a ReSpec / CSS issue. This paragraph is defined as a note with a title, but the note style is not in fact styled in a distinctive way. Nick: now fixed in ReSpec 2.1.26 - but using whatever style was previously there for notes, issues, and comments. The OP members should review that style, and raise new ReSpec issues for any necessary changes.
• [x] 5. /§3/ "RDF statement like the following (rendered here in Turtle)" is redundant, you mentioned in §1.2 that all examples will follow in Turtle.
• [x] 6. /§3/ I don't think the Root Services document is a proper RDF document and the trs:trackedResourceSet property advice would apply to it? Nick: I've removed the reference to root services.
  1. Unless we plan to follow through on the Add all Root Services properties to SPC shape · Issue #467 · oslc-op/oslc-specs · GitHub ticket. But that can only happen in OSLC Core 3.1.
  2. /§3/ I think we should say that the OSLC server SHOULD point to a TR Sets from the well-known SPC. Nick: I'm not happy adding that statement. A catalog points only to service providers, and a TRS is not currently defined as a service in its own right. We could define entries in a service to point to a TRS, but as above, that would require a Core 3.1 update.
• [x] 7. /§4/ I think §3 Discovery shall not come before §4 General Behaviour. Nick: I'm not sure I agree. Discovering a service comes before trying to work with it!
• [x] 8. /§4/ Shouldn't we require conformance with either Core 2 or Core 3 but not both at the same time? Nick: It says 'or'
• [ ] 9. /§4/ I don't think CC-5 "The TRS Server SHOULD support ETags, caching, and conditional GETs for those resources." is sufficiently described. Nick: Aren't these standard HTTP constructs? Why should they need defining further here?
• [x] 10. /§5.2/ looks like the shape will apply to ALL LDP Containers. Also may be good to name it §5.2 TRS Base
• [x] 11. /§5.3/ may be good to name it §5.3 TRS Change Log &c. for the sections below.
• [x] 12. /§5/ Where is a trs:ChangeEvent that is a superclass of Deletion, Modification, and Deletion. Nick: There is no such superclass.
• [x] 13. /§5.4/ CC-13 is not enough, newer events SHALL NO have an order value less than that of an older event.
• [x] 14. /§6/ can CC-18 be split up into something more readable? E.g. A trs:cutoffEvent indicates that the TRS Base lists tracked resources present in the system at the point right after a given event.
• [x] 15. /§6/ What is "gap between the Base portion and the Change Log portion of a Tracked Resource Set representation." supposed to mean? Also, you used the word "gap" to refer to the trs:order being strictly monotonic but not necessarily sequential. Let's use one term to denote one thing if possible.
• [x] 16. /§7/ discusses Change Events without defining a shape for them or referring to the vocal or introducing them (the Terminology section doesn't count as it's non-normative). Nick: Fixed by expanding the terminology section on this topic, and making that section normative.
• [x] 17. /§7/ What does it mean "The ordering of Change Log events is meaningful for changes to any specific resource, but there is no meaning to the ordering of change log events for different resources.". Also see my comment no. 13.
• [x] 18. /§7/ Was no. 17 statement present in the TSR 2.0 Final draft or was this relaxation added just now in 3.0? Nick: I added this relaxation to address more sophisticated multi-threaded servers.
• [x] 19. /§7/ "The Change Log does not necessarily identify every change of internal state" – what does "internal state" mean here? I am interested in how it maps (or not) to the OSLC Resources publicly visible to the OSLC Clients. Nick: I have removed the word 'internal' - the point is not limited to states that are not externally visible.
• [x] 20. /§7/ CC-23 sounds non-normative to me. Maybe make some statements that TRS CEs MAY be delayed after OSLC TRs have been modified? Nick: I cannot now see what this comment references - I have scoped down some of the conformance markers to be more precise.
• [x] 21. /§7/ I am interested to know if CC-23 means that a delay in CE publication means that other newer CEs generated after a given delayed one will not be published until the delayed one is visible. Nick: That would be subject to the other ordering constraints. I do not believe additional spec text is required.
• [x] 22. /§7/ CC-24 is unnecessarily complicated yet does not provide any timing/causality guarantees. Nick: I do not understand the issue.
• [x] 23. /§7/ CC-26 is easy to handle but I think it shall be removed unless we have a good motivation. Nick: I do not understand the issue.
• [ ] 24. /§7/ CC-27 makes no sense whatsoever. Nick: I do not understand the issue.
• [ ] 25. /§7/ CC-28 makes no sense whatsoever. Nick: I do not understand the issue.
• [ ] 26. /§7/ CC-30 is inappropriate. Nick: I do not understand the issue.
• [ ] 27. /§7/ "A Base might omit mention of a Resource that ought to have been included or include a Resource that ought to have been omitted." (a) shall be a CC (MAY omit). (b) this is #$%^! and contradicts CC-19. Nick: I do not agree.
• [x] 28. /§7/ "From the above sections, one can infer that the consistency and completeness of the change log with respect to the date on a TRS Server is only possible after reading the entire Base and Change Log." Actually no, everything I have read so far tries to indicate that even if I read the entire Base and CL and encounter no problems, all bets are still off! Nick: Not all bets - just that a TRS will never guarantee a completely up to date picture - it can always lag. I have upated the wording to say this.
• [x] 29. /§7/ "A client cannot skip the initial reading of the base, nor partially consume a Change Log, and then expect consistency with some earlier point-in-time of the server’s data." sounds non-normative. Nick: And it is. - but this was not visible until the update to ReSpec 2.2.26.
• [ ] 30. /§8/ "each one containing successively lower ordered events" is not sufficiently defined. cf. (13) and other comments above around ordering. Nick: I do not understand the issue. This might have been addressed by my other changes.
• [x] 31. /§8/ Suggestion to reword CC-35: "that any event in any preceding Change Log segment". "Than any events in all" does not sound right.
• [x] 32. /§8/ "TRS Servers must include the Change Events for each Change Log segment in the HTTP response for the Change Log segment. " What is that supposed to mean? Water SHALL contain water?
• [x] 33. /§12/ Where is example 1?
• [x] #516 Nick: split out to new open issue.
• [x] 35. /§13/ Is CC-55 correct? Is it not "MUST be sep by at least 1 whitespace char"? Nick: The text is correct as-is. White space is not required.
• [x] 36. /§13/ CC-56 "MUST be the ASCII character 'A' or 'D'" or "MUST be the character 'A' (U+0041) or the character 'D' (U+0044)". Greek Α (U+0391) or Cyrillic А (U+0410) is unlikely to make you happy.

[ndjc]

I will start addressing these comments in a new branch b-trs30-draft3, as discussed in the meeting on May 20th. I do not expect all to be resolved in that new draft, so I've added a checklist above, and will mark off the mainly editorial actions as I make them. I will probably leave some open to be discussed later - the topic of discovery, root services, etc. is one of those.

[berezovskyi]

@jamsden do you still want to go over these items?

[jamsden]

Yes

[jamsden]

We are about to publish trs-v3.0-ps01. @berezovskyi, are the any of these items that should be addressed in ps01?

[berezovskyi]

Jim, I am on vacation this week. The most important issue is to remove the language that allows items to appear out of order, e.g. 5 7 8 9 6 (ie 5 on the first request, 5 7 on the second, then 5 7 8, then 5 7 8 9 and finally 5 7 8 9 6). This may be issues 21, 27, 30 but I am not sure unless I cross-check with the draft.

Our client triggers a full rebase every time it encounters such a SNAFU. Removing text from the spec that condones such behavior would allow users to go their vendors and file bug reports asking for events not to be published out of order but at the same time clients will still have the code that works around this.

[berezovskyi]

Note that this has nothing to do with another issue I have that 5 7 8 9 10 is allowed. Better to have skips like 5 7 8 9 10 than OOOE like 5 7 8 9 6.

[berezovskyi]

Some systems deal with this through watermarking, see https://databricks.com/blog/2017/05/08/event-time-aggregation-watermarking-apache-sparks-structured-streaming.html but we don't want to go there. At the very least, we need to ensure that the order of trs:order matches the TRS log append order (no pun intended).

I recall David and Nick arguing that it's "hard" to do in a clustered environment. As I said before, AtomicLong allows to generate 1MM+ TRS event order numbers per second and Hazelcast/Redis allow at least 300k atomic INCR commands across multiple nodes in a cluster on a decent server HW. I am pretty sure a SQL DB with an AUTO INCREMENT key will do the trick, even if 10x slower. I am 100% confident we can enforce both constrains but here specifically we need to deal with out-of-order appends.

[jamsden]

re: "language that allows items to appear out of order", I don't see any language that allows or implies that. Can you point to that language?

[berezovskyi]

1. CC-25
2. CC-23 is suspicious (it talks about changes, but it's not clear if we are talking about change time or changelog append time; the reorder of the former is fine but not the latter)
3. CC-17 talks about "recency" of the events, not their append order. In its form it directly contradicts CC-25.

We should define two separate terms: TR event time and TRS event append time. And then restate those CCs, such that two TR events may appear out of order in the TRS (CC-23), state that TR events may be delayed from being appended to the TRS (CC-25) but once a TRS Client fetches the TRS log and observes a set of TRS events with a trs:order of up to n, no subsequent TRS log responses may contain appended TRS events with a trs:order < n that were missing from a previous response (CC-17). This would mean that if CC-23/25 happen, they will have TRS event order numbers as they are appended to the log, not according to the original TR event time order.

[berezovskyi]

For now the only spelled out guarantee is in CC-36, which says that when you get a new page, it will not contain events with an order number lower or equal to the max order number from a previous page.

[jamsden]

CC-25: states under what conditions a TRS server must report a change event, it doesn't address the ordering of the events in the change log.

CC-23: indicates that change log ordering is independent for different resources, the change log order is only relevant for changes to a specific resource - in oder to ensure the state of the resource is accurately reflected over time.

CC-14, CC-17: indicate that the trs:order number sequence must correspond to the actual sequence of changes to the trs:changed resource. But does not say anything about the order of these events in the change log. They might for example appear out of order if members of clustered servers appended to the log in an asynchronous manner.

The client however can always rely on the trs:order numbers, and should not rely on their order in the change log. If they want to process them in the right order, one way would be to sort them before processing them.

So I'm still not sure any change in the spec is needed.