istio / old_mixer_repo

Deprecated home of Istio's Mixer and its adapters, now in istio/istio's mixer dir
https://github.com/istio/istio/tree/master/mixer
Apache License 2.0
67 stars 93 forks source link

Generate the global word list from the source YAML file rather than having it hardcoded in Mixer #811

Open geeknoid opened 7 years ago

geeknoid commented 7 years ago

In Mixer, pkg/api/globalWordList.go contains the current global word list. This is a copy of a file in istio/api/mixer/v1/global_word_dictionary.yml. It'd be great to generate Mixer's go file directly from what's in istio/api/...

ZackButcher commented 7 years ago

Rather than generate a go file from the yaml I'd rather see Mixer ingest the global dictionary as a yaml file directly. Eventually we'll want to handle serving with multiple versions to make it possible to roll out updates incrementally, so we're going to have to dump the hard coded version anyway.

geeknoid commented 7 years ago

Give the rules on how the dictionary must be updated (at the end only), I don't want to make this an easily edited YAML file. Not supplying or shortening the list could substantially reduce Mixer throughput too.

I'd rather have this baked in to code. I expect this to have a tremendously slow update frequency. We could in fact chose to never update the dictionary if we wanted to.

geeknoid commented 7 years ago

Some additional perspectives.

Updating the dictionary is a relatively rare thing. Having the dictionary in the binaries, rather than read from a config file or some sort, means that updating the dictionary involves updating binaries.

We will already have procedures in place for the operator to update our binaries. There will invariable be ordering dependencies that we will describe and perhaps validate. The operator will necessarily be involved in binary upgrades at some level or another. We will have to document and support the binary upgrade process.

If the dictionary were exposed as a distinct file, then it would be yet another thing that operators need to deal with it. It would have its own upgrade process, it's own docs, it's own validation mechanisms, etc. It would increase the load on operators.

So given the rare upgrades to the dictionary, I much rather hide the existence of the dictionary in the binaries and depend on binary rollout to deal with dictionary upgrades.

BTW, the model we've set up means that you upgrade Mixer first. Then you can upgrade all the clients lazily, as the Mixer automatically handles older dictionaries. Mixer will fail fast however if a client sends it requests using a newer (larger) dictionary than it has.

We could consider implementing a retry protocol in mixerclient such that a request fails due to the Mixer complaining it doesn't understand some words in the dictionary, mixerclient could automatically downgrade itself such that subsequent calls use a smaller dictionary.

@qiwzhang Do you think we should proceed to enhance mixerclient such that it can detect when its dictionary is too new and then backoff to a smaller dictionary for all future requests?

qiwzhang commented 7 years ago

We could. But mixer has to reply with number of words it has. client can only use min (all mixer replies).

geeknoid commented 7 years ago

I was thinking that in case of failure, mixerclient would fall back to the V1 global dictionary which is the baseline. It would continue to use V1 until it is rebooted. So any failure to use a more modern dictionary would immediately cause the client to degenerate to the original dictionary.

On Fri, Jun 9, 2017 at 12:55 PM, Wayne Zhang notifications@github.com wrote:

We could. But mixer has to reply with number of words it has. client can only use min (all mixer replies).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/istio/mixer/issues/811#issuecomment-307484789, or mute the thread https://github.com/notifications/unsubscribe-auth/AVucHdFkt5jeyaaDNncVDMgpluHhFbA2ks5sCaM3gaJpZM4N1oGp .

qiwzhang commented 7 years ago

V1 is the first version with 111 words? Yes, we could do that. just lost some compression.