matching: metrics related fixes

[venkat1109]

Follow patch to #1906. The domainScope was assigned within a lock however it was accessed throughout the code without acquiring any locks. This patch fixes the bug and also gets rid of the locks, in favor of atomics.

There was also another existing bug - the metricsClient was never initialized in the constructor. Also, fixes this problem

[longquanzheng]

If what you concern about race condition, let's revert my previous PR because what you are doing here is exactly the same thing as DomainCache does. See https://github.com/uber/cadence/blob/master/common/cache/domainCache.go#L89
So it doesn't improve much perf compared to directly using domainCache. Instead, it over complicates things because you are building another cache over cache.

[venkat1109]

So it doesn't improve much perf compared to directly using domainCache
This patch caches the domain metric scope once it's created not the domain metadata. That's about it. DomainCache caches the domain metadata. And I tend to disagree that it's complicated.

[longquanzheng]

So it doesn't improve much perf compared to directly using domainCache
This patch caches the domain metric scope once it's created not the domain metadata. That's about it. DomainCache caches the domain metadata. And I tend to disagree that it's complicated.

I am fine to forget about complexity as it's hard to argue. Adding another atomicValue is unnecessary. How about just use domainCache whenever you need to translate from domainID to domainName?

[venkat1109]

So it doesn't improve much perf compared to directly using domainCache
This patch caches the domain metric scope once it's created not the domain metadata. That's about it. DomainCache caches the domain metadata. And I tend to disagree that it's complicated.

I am fine to forget about complexity as it's hard to argue. Adding another atomicValue is unnecessary. How about just use domainCache whenever you need to translate from domainID to domainName?

Discussed offline.