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

When renaming a host in TC, the hashId retains the value of the previous hostname #2156

Closed
guzzijason opened this issue Apr 23, 2018 · 10 comments · Fixed by #4881
Closed

When renaming a host in TC, the hashId retains the value of the previous hostname #2156

guzzijason opened this issue Apr 23, 2018 · 10 comments · Fixed by #4881
Labels
bug something isn't working as intended Traffic Ops related to Traffic Ops

Comments

@guzzijason
Copy link
Contributor

Observed in TP 2.3.0-8247.fb69f019.el7

Possible hashId naming collision if previous hostname value is retained.

@mitchell852
Copy link
Member

This appears to be because in the crconfig.json file, the hashId is derived from the server's xmpp_id if it exists and the server's host_name if it doesn't. So in the cases where xmpp_id exists, a hostname change will not affect the hashId.

https://github.com/apache/incubator-trafficcontrol/blob/master/traffic_ops/app/lib/UI/Topology.pm#L280

@mitchell852 mitchell852 added Traffic Ops API bug something isn't working as intended labels Apr 23, 2018
@jhg03a
Copy link
Contributor

jhg03a commented May 23, 2018

An alternative solution might be write once change never on server hostname/domain

@mitchell852
Copy link
Member

I "think" @dewrich fixed this. If so @dewrich could you reference this issue from your PR?

@dewrich
Copy link
Contributor

dewrich commented Jul 23, 2018

@mitchell852 Are you referring to this one? #2345

@mitchell852 mitchell852 added Traffic Ops related to Traffic Ops and removed Traffic Ops related to Traffic Ops Traffic Ops API labels Oct 17, 2018
@mitchell852 mitchell852 removed the intake label Nov 7, 2018
@mitchell852
Copy link
Member

mitchell852 commented Jun 11, 2020

Just to add some more details to this:

How to reproduce:

  1. Perform a snapshot of your cdn so the diff is clean: https://tp.domain.com/#!/cdns/{id}/config/changes
  2. Find an existing Edge server in that cdn (because only edge servers show up in a snapshot) to update. i.e. https://tp.domain.com/#!/servers/{id}
  3. change the hostname of the server and save.
  4. go back to the cdn snapshot and observe the changes that are being made to the Traffic Servers (contentServers) portion of the snapshot. notice that the hashId did not change with the hostname change.

The point is if you create a server with hostname = foo, it ends up with hashId = foo. when you change hostname to bar, hashId stays foo. hashId is set in stone...which is a good thing as far as TR and consistent hashing is concerned.

However, this means you can come along and now create another server with hostname=foo (because you renamed the other one to bar remember?) and you end up with 2 servers with hashId=foo which means TR could get confused with its consistent hashing algo and never send traffic to one of the servers and this would probably be tough to figure out why one server never gets any traffic.

So, I would suggest that simply making hostname immutable on a server would prevent this scenario from every happening.

@mitchell852
Copy link
Member

also, not sure if it should be part of this ticket or not but any/all references to xmpp_id and xmpp_password should be stripped out of TO entirely as it's dumb and confusing. :)

@rawlinp
Copy link
Contributor

rawlinp commented Jun 11, 2020

Alternatively, when a server is created, we could set the XMPPID to a randomly-generated UUID. In my opinion, we should never be able to choose or change the XMPPID. This would prevent hashIds from ever colliding but still allow the hostname to be mutable.

@mitchell852
Copy link
Member

ok, yeah, i guess XMPPID could be used for that purpose which gives it some value i guess.

@ocket8888
Copy link
Contributor

That is what XMPPID is currently used for, I think, storing the hash id.

but IMO the name should be changed to "Hash ID"

@rawlinp
Copy link
Contributor

rawlinp commented Jun 11, 2020

Should be, but that would just be another breaking change to maintain differently between api v1/v2/v3. Given the sensitivity of this field (in terms of how it relates to consistent hashing on a CDN) I'd be more comfortable leaving it as-is (XMPPID). Maybe we should save that breaking change for another version once we've cleared out most of the v1/v2 cruft.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something isn't working as intended Traffic Ops related to Traffic Ops
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants