Deadlock scenario:
1. Due to scheduling, the state runner sends one snapshot into
snapCh and then attempts to send a second. The first send succeeds
because the channel is buffered, but the second blocks.
2. Separately, Manager.Watch is called by the xDS server after
getting a discovery request from Envoy. This function acquires the
manager lock and then blocks on receiving the CurrentSnapshot from
the state runner.
3. Separately, there is a Manager goroutine that reads the snapshots
from the channel in step 1. These reads are done to notify proxy
watchers, but they require holding the manager lock. This goroutine
goes to acquire that lock, but can't because it is held by step 2.
Now, the goroutine from step 3 is waiting on the one from step 2 to
release the lock. The goroutine from step 2 won't release the lock until
the goroutine in step 1 advances. But the goroutine in step 1 is waiting
for the one in step 3. Deadlock.
By making this send non-blocking step 1 above can proceed. The coalesce
timer will be reset and a new valid snapshot will be delivered after it
elapses or when one is requested by xDS.
* Use NodeName not Node for cross checking proxies/instances
* Also copy over the meta data to keep the correct cursor/index
* When we sync checks to the ProxyInstance replace rather than accumulate
This commit makes a number of changes that should make
TestLoad_FullConfig easier to work with, and make the test more like
real world scenarios.
* use separate files in testdata/ dir to store the config source.
Separate files are much easier to edit because editors can syntax
highlight json/hcl, and it makes strings easier to find. Previously
trying to find strings would match strings used in other tests.
* use the exported config.Load interface instead of internal NewBuilder
and BuildAndValidate.
* remove the tail config overrides, which are only necessary with
nonZero works.
This commit reduces the interface to Load() a bit, in preparation for
unexporting NewBuilder and having everything call Load.
The three arguments are reduced to a single argument by moving the other
two into the options struct.
The three return values are reduced to two by moving the RuntimeConfig
and Warnings into a LoadResult struct.
There are many places in the API where we receive a property set to
`null` which can then lead to defensive code deeper in the app in order
to guard for this type of thing when usually we are expecting an array
or for the property to be undefined using omitempty on the backend.
Previously we had two places where we would deal with this in the
serializer using our 'remove-null' util (KV and Intentions).
This new decorator lets you declaritively define this type of data using
a decorator @NullValue([]) (which would replce a null value with [].
@NullValue in turn uses a more generic @replace helper, which we
currently don't need but would let you replace any value with another,
not just a null value.
An additional benefit here is that the guard/replacement is executed
lazily when we get the property instead of serializing all the values
when they come in via the API. On super large datasets, where we only
visualize part of the dataset (say in our scroll panes), this feels like
a good improvement on the previous approach.
Alvin Huang