Add timespec structured mtime metadata #236

ribasushi · 2019-12-23T17:30:10Z

Augment the mtime specification with a well-defined universal datastructure capable of expressing both arbitrarily distant points in time, and sufficiently arbitrary ( in computing terms ) sub-second precision. The very slight increase in on-wire size and the mild extra parsing complexity allows for:

A structure representing time, that is unlikely to change any time in the future, aiding implementation convergence
A way to encode any filesystem-related timestamp within the wider problem domain of information systems

UNIXFS.md

achingbrain · 2019-12-23T17:36:41Z

UNIXFS.md

@@ -90,7 +103,9 @@ UnixFS currently supports two optional metadata fields:
  - The remaining 20 bits are reserved for future use, and are subject to change. Spec implementations **MUST** handle bits they do not expect as follows:
    - For future-proofing the (de)serialization layer must preserve the entire uint32 value during clone/copy operations, modifying only bit values that have a well defined meaning: `clonedValue = ( modifiedBits & 07777 ) | ( originalValue & 0xFFFFF000 )`
    - Implementations of this spec must proactively mask off bits without a defined meaning in the implemented version of the spec: `interpretedValue = originalValue & 07777`
-* `mtime` -- The modification time in seconds since the epoch. This defaults to the unix epoch if unspecified
+* `mtime` -- A two-element structure ( `EpochSeconds`, `EpochNanoseconds` ) representing the modification time in seconds relative to the unix epoch `1970-01-01T00:00:00Z`. In contexts where an mtime is mandatory ( e.g. FUSE interfaces ) implementations must treat an unspecified mtime as `0`.


I think we should just default this to 0 as it is currently. The go-ipfs and js-ipfs implementations cannot control which context they will be used from so should default to the most conservative approach so that might as well be in the spec.

My hesitation placing this in the spec is that it implicitly means unknown mtime is not a thing. E.g. a web gateway would be obligated to send out a 0 Last-Modified header, and so on...

What do you think?

I think that's fine. At least, I can't think of a reason that makes it not fine, given that we need a default value and we're happy with seeing 1970 everywhere when we ipfs files ls -l.

To make this more concrete, I think a web gateway sending a 0 Last-Modified header is ok.

:(

I will relent if this is a wider consensus, but then it makes me sad as this part of the spec becomes unusable to me as well. In my use case having a gateway render "this is the mtime" vs "mtime was not supplied" is really crucial.

UNIXFS.md

alanshaw · 2019-12-26T11:10:47Z

Please can we have some context in the PR description to explain to people why this is required?

achingbrain · 2019-12-26T14:08:20Z

Not sure I understand the maths behind why they’ll often be this big?

2^28 is roughly 1/4 of the total range of nanosecond values so assuming a uniform distribution they'll be bigger than that 3/4 of the time.

UNIXFS.md

achingbrain · 2020-01-02T07:00:02Z

UNIXFS.md

@@ -90,7 +103,9 @@ UnixFS currently supports two optional metadata fields:
  - The remaining 20 bits are reserved for future use, and are subject to change. Spec implementations **MUST** handle bits they do not expect as follows:
    - For future-proofing the (de)serialization layer must preserve the entire uint32 value during clone/copy operations, modifying only bit values that have a well defined meaning: `clonedValue = ( modifiedBits & 07777 ) | ( originalValue & 0xFFFFF000 )`
    - Implementations of this spec must proactively mask off bits without a defined meaning in the implemented version of the spec: `interpretedValue = originalValue & 07777`
-* `mtime` -- The modification time in seconds since the epoch. This defaults to the unix epoch if unspecified
+* `mtime` -- A two-element structure ( `EpochSeconds`, `EpochNanoseconds` ) representing the modification time in seconds relative to the unix epoch `1970-01-01T00:00:00Z`. In contexts where an mtime is mandatory ( e.g. FUSE interfaces ) implementations must treat an unspecified mtime as `0`.


I think that's fine. At least, I can't think of a reason that makes it not fine, given that we need a default value and we're happy with seeing 1970 everywhere when we ipfs files ls -l.

UNIXFS.md

Stebalien

I'm not sure if it's really worth avoiding an extra tag, but this seems like a clean solution anyways.

- Settle on UnixTime, remove mention of epoch entirely - Better explain fixed32 rationale for the fractional part

ribasushi · 2020-01-07T15:25:26Z

Ok so... this tortured PR is now blocked on last two issues:

Range of the fractional nanosecond value. I read this as protocol-level spec, I am advocating for "0 nanoseconds" to be an invalid value. @achingbrain comes from implementation point of view, and correctly points out that a user should not care whether a value is 0 or not ( feat: store time as timespec js-ipfs-unixfs#40 (comment) )
- As resolution I propose adding verbiage to the spec that spells out what is valid on the wire vs what is valid on the API side. I have not added to this PR, as there seems to be aversion to a wordy doc
The question over whether "no mtime" automatically implies 0 for all intents and purposes, or whether implementations ( e.g. a web gateway ) may elect to render actual 0 and "no data" in distinct ways. @achingbrain advocates for 0/1970-01-01 to be mandated by the spec, including web-gateways being obligated to always supply a Last-Modified: header. I find this unworkable for the archival use-cases I have in mind, and am strongly for leaving this at the implementation's discretion ( Add timespec structured mtime metadata #236 (comment) )
- Resolution is difficult, as the outcome is zero-sum. I guess requesting for @alanshaw and @Stebalien to break the tie ;)

Stebalien · 2020-01-07T17:52:35Z

As resolution I propose adding verbiage to the spec that spells out what is valid on the wire vs what is valid on the API side. I have not added to this PR, as there seems to be aversion to a wordy doc

I agree that we should not encode 0 nanoseconds on the wire.

The question over whether "no mtime" automatically implies 0 for all intents and purposes, or whether implementations ( e.g. a web gateway ) may elect to render actual 0 and "no data" in distinct ways.

I'd let implementations decide what to do here. However, we should specify what they can do. They can either:

Preferred: indicate that the time is unset.
Otherwise: return the epoch. Unfortunately, some APIs make it difficult to indicate that no mtime is set.

That is, they can't just return any random time. They should also document what they do.

Stebalien · 2020-01-07T21:12:22Z

Merging per @ribasushi's suggestion as we can address the final two points in a followup PR.

achingbrain · 2020-01-08T11:27:46Z

On mtime:

@achingbrain advocates for 0/1970-01-01 to be mandated by the spec

The reason for this is primarily this comment and that prior metadata PRs have specified a default value.

I am more than happy to make this field optional, and solid use cases for this definitely strengthen that argument. It also means the mtime field in the protobuf definition doesn't need extra exposition around it which is good too.

Addresses #236 (comment)

ribasushi · 2020-01-08T13:54:50Z

@achingbrain @Stebalien see #237 for the next attempt at expressing this.

Attempt to address mtime remnants of #236

Add timespec structured mtime metadata

f993894

ribasushi requested review from Stebalien and achingbrain December 23, 2019 17:30

achingbrain reviewed Dec 23, 2019

View reviewed changes

UNIXFS.md Outdated Show resolved Hide resolved

achingbrain reviewed Dec 23, 2019

View reviewed changes

UNIXFS.md Outdated Show resolved Hide resolved

achingbrain reviewed Dec 23, 2019

View reviewed changes

UNIXFS.md Outdated Show resolved Hide resolved

achingbrain reviewed Dec 23, 2019

View reviewed changes

UNIXFS.md Outdated Show resolved Hide resolved

Address several comments

3205ce5

ribasushi requested a review from achingbrain December 23, 2019 18:15

alanshaw requested changes Dec 26, 2019

View reviewed changes

UNIXFS.md Outdated Show resolved Hide resolved

UNIXFS.md Outdated Show resolved Hide resolved

achingbrain reviewed Jan 2, 2020

View reviewed changes

This was referenced Jan 2, 2020

docs: support high resolution mtime metadata #235

Closed

feat: store time as timespec ipfs/js-ipfs-unixfs#40

Merged

Stebalien approved these changes Jan 2, 2020

View reviewed changes

ribasushi requested a review from alanshaw January 6, 2020 14:41

alanshaw approved these changes Jan 7, 2020

View reviewed changes

Multiple fixes as discussed:

da9fa64

- Settle on UnixTime, remove mention of epoch entirely - Better explain fixed32 rationale for the fractional part

Stebalien approved these changes Jan 7, 2020

View reviewed changes

Stebalien merged commit b9ca0be into master Jan 7, 2020

Stebalien deleted the optional_hires_mtime2 branch January 7, 2020 21:12

ribasushi added a commit that referenced this pull request Jan 8, 2020

Attempt to address mtime remnants of #236

8888281

Addresses #236 (comment)

ribasushi mentioned this pull request Jan 8, 2020

Attempt to address mtime remnants of #236 #237

Merged

Stebalien added a commit that referenced this pull request Jan 9, 2020

Merge pull request #237 from ipfs/optional_hires_mtime3

3816b05

Attempt to address mtime remnants of #236

Add timespec structured mtime metadata #236

Add timespec structured mtime metadata #236

Uh oh!

Conversation

ribasushi commented Dec 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

achingbrain Dec 23, 2019

Choose a reason for hiding this comment

Uh oh!

ribasushi Dec 23, 2019

Choose a reason for hiding this comment

Uh oh!

achingbrain Jan 2, 2020

Choose a reason for hiding this comment

Uh oh!

achingbrain Jan 6, 2020

Choose a reason for hiding this comment

Uh oh!

ribasushi Jan 6, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alanshaw commented Dec 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

achingbrain commented Dec 26, 2019

Uh oh!

Uh oh!

achingbrain Jan 2, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Stebalien left a comment

Choose a reason for hiding this comment

Uh oh!

ribasushi commented Jan 7, 2020

Uh oh!

Stebalien commented Jan 7, 2020

Uh oh!

Stebalien commented Jan 7, 2020

Uh oh!

achingbrain commented Jan 8, 2020

Uh oh!

ribasushi commented Jan 8, 2020

Uh oh!

Uh oh!

ribasushi commented Dec 23, 2019 •

edited

Loading

alanshaw commented Dec 26, 2019 •

edited

Loading