Skip to:

  1. Skip to Jira Navigation
  2. Skip to Side Navigation
  3. Skip to Main Content

Fix problems with HPCC-12516

Description

The PR for contains various problems:

  • The system doesn't compile if USE_LIBMEMCACHED is not defined. (HPCC-17797)

  • Various functions are not thread safe. E.g., ensureMemCachedClient(), addMemCachedGlobal, addMemCachedSeconds { setCacheTimeout(value, global) would be a cleaner interface }

  • It wasn't obvious how the timeouts were implemented.

  • queryMemCacheSeconds is casting to unsigned, but returning an int. That will cause warnings on windows.

Also, the interface should be more abstract that it is. memcached is an implementation detail - it definitely shouldn't be included in the names of the member functions. What happens if we switch from memcached to another implementation?

memCachedClient should be a pointer to a caching interface.
ESPMemCached should be an implementation of a general caching interface. There shouldn't be any references to memcached in httptransport.hpp/ipp/cpp.

Conclusion

None

Activity

Show:

Kanghua Wang July 14, 2017 at 7:36 PM

The intention is to push (not force) people to use ESP cache (not have to be memcached). But, for now, ESP does only support memcached as an ESP cache method. In the PR (https://github.com/hpcc-systems/HPCC-Platform/pull/10225), we give a user an option to not use ESP cache.

Cc

Yanrui Ma July 14, 2017 at 7:25 PM
Edited

AttilaAttila 12:32
If I see and understand well our ESP server has a hard dependency with memcached server. My question is do we install and start that server along with our platform? If not, why not? (We broke our out-of-box system if somehow the memched doesn't exists in the target environment.) If yes, then I'm sorry for this disturbance.
GavinGavin
Not answering your question, but asking a different one. Do we really want this as a hard dependency - the functionality should be optional and configurable to use other implementations. Did a jira get opened for that?

Kanghua Wang June 16, 2017 at 3:08 PM
Edited

  • I think that both addMemCachedGlobal and addMemCachedSeconds do not need thread safe because they are called only when ESP is starting and the WsWorkunits lib is loading. But, ensureMemCachedClient() does. I will add CriticalBlock block(memCachedCrit).

  • The queryMemCacheSeconds() will be revised to avoid the warnings on windows.

  • The implementation of a general caching interface is a great idea. Will do.

Fixed
Pinned fields
Click on the next to a field label to start pinning.

Details

Components

Assignee

Reporter

Priority

Fix versions

Pull Request URL

Created June 12, 2017 at 1:39 PM
Updated July 27, 2017 at 8:18 AM
Resolved July 27, 2017 at 8:18 AM
Loading...