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

replicas ttl are possibly different #864

Closed
PapaYofen opened this issue Feb 26, 2019 · 5 comments
Closed

replicas ttl are possibly different #864

PapaYofen opened this issue Feb 26, 2019 · 5 comments

Comments

@PapaYofen
Copy link
Contributor

Sponsors SeaweedFS via Patreon https://www.patreon.com/seaweedfs

Describe the bug
ReplicatedWrite do NOT fill ttl paramemter so ttl of replicas is possibly different with the first written one, in light of the following codes
https://github.com/chrislusf/seaweedfs/blob/344caf3cd7d2de00469a61ae3bf597b8e9bab726/weed/topology/store_replicate.go#L48
And setting ts when replicatedWrite makes volume servers strongly depend on ntp
https://github.com/chrislusf/seaweedfs/blob/344caf3cd7d2de00469a61ae3bf597b8e9bab726/weed/topology/store_replicate.go#L52
Expected behavior

  • ttl of all replicas are the same
  • remove ts when ReplicatedWrite
PapaYofen added a commit to PapaYofen/seaweedfs that referenced this issue Feb 26, 2019
@chrislusf
Copy link
Collaborator

  1. ttl is determined when allocating a volume. It is not used when writing a file.
  2. I do not understand why need to remove the timestamp information.

@PapaYofen
Copy link
Contributor Author

PapaYofen commented Feb 27, 2019

  1. ttl is determined when allocating a volume. It is not used when writing a file.

According to replicas logic and codes, when writting a file, ttl can be set in url (e.g. ttl=1m) the 1st written one(aka needle) has the ttl property, others has no needle ttl property
It causes ambiguity that 1st wirtten one strictly follow the needle's ttl property when others follows the volume's ttl
And this ambiguity may lead the side effect that clients sometimes can read the file , sometimes cann't.

  1. I do not understand why need to remove the timestamp information.

if different volumes run on different machines, not sync-ing time by ntp, the replica volume's lastModifiedTime will be set same to the main volume server's, it also causes ambiguity.

e.g.

2 replicas: ttl = 5m
time of volume-0: 2019-02-27T00:00:00.000Z
time of volume-1: 2019-02-27T00:04:00.000Z

when write file in volume-0 at 2019-02-27T00:00:00.000Z, then write replica on volume-1, last modifed time of volume-1 will also be set as ts=2019-02-27T00:00:00.000Z, then after 1m volume-1 will be removed.

@chrislusf
Copy link
Collaborator

  1. the fix should be good.
  2. I disagree. The initial time is the event time and should not change. Other machine has wrong time is not a reason to change that.

@PapaYofen
Copy link
Contributor Author

  1. the fix should be good.
  2. I disagree. The initial time is the event time and should not change. Other machine has wrong time is not a reason to change that.

ok, that is to say seaweedfs depends on ntp.
So maintaining ts and adding ttl is ok? like

q := url.Values{
	"type": {"replicate"},
	"ttl":  {needle.Ttl.String()},
}
if needle.LastModified > 0 {
	q.Set("ts", strconv.FormatUint(needle.LastModified, 10))
}

@chrislusf
Copy link
Collaborator

this change should be good.

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

No branches or pull requests

2 participants