Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a memcached-based L1 caching layer to improve API performance by adding an in-memory cache tier above the existing Redis cache system.
Key changes:
- Adds memcached service to Docker Compose setup with appropriate configuration
- Implements a two-tier caching strategy (L1 memcached, L2 Redis) in the cache middleware
- Creates a dedicated MemCache utility class for memcached operations
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| docker-compose.yml | Adds memcached service configuration and dependency |
| config/cache.php | Updates memcached configuration to use UNIX socket for performance |
| app/Utils/Cache/MemCache.php | New utility class providing memcached operations with region tracking |
| app/Http/Middleware/CacheMiddleware.php | Implements L1/L2 caching strategy with memcached as primary tier |
| app/Services/Utils/RedisCacheService.php | Integrates memcached clearing with Redis cache region clearing |
| app/Services/Model/Imp/ProcessScheduleEntityLifeCycleEventService.php | Adds debug logging for cache operations |
| Dockerfile | Adds memcached PHP extension |
| .github/workflows/push.yml | Adds memcached service and configuration for CI |
| .env.example | Adds memcached environment variables |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| 'port' => env('MEMCACHED_PORT', 11211), | ||
| 'weight' => 100, | ||
| 'host' => env('MEMCACHED_SERVER_HOST', '/var/run/memcached/memcached.sock'), | ||
| 'port' => env('MEMCACHED_SERVER_PORT',0), |
There was a problem hiding this comment.
Missing space after comma in function call. Should be env('MEMCACHED_SERVER_PORT', 0) for consistency with Laravel coding standards.
| 'port' => env('MEMCACHED_SERVER_PORT',0), | |
| 'port' => env('MEMCACHED_SERVER_PORT', 0), |
| 'weight' => 100, | ||
| 'host' => env('MEMCACHED_SERVER_HOST', '/var/run/memcached/memcached.sock'), | ||
| 'port' => env('MEMCACHED_SERVER_PORT',0), | ||
| 'weight' => env('MEMCACHED_SERVER_WEIGHT',100) |
There was a problem hiding this comment.
Missing space after comma in function call. Should be env('MEMCACHED_SERVER_WEIGHT', 100) for consistency with Laravel coding standards.
| 'weight' => env('MEMCACHED_SERVER_WEIGHT',100) | |
| 'weight' => env('MEMCACHED_SERVER_WEIGHT', 100) |
|
|
||
| $encoded = Cache::tags($regionTag) | ||
| ->remember($key, $cache_lifetime, function() use ($next, $request, $regionTag, $key, $cache_lifetime, &$status,$ip, $agent) { | ||
| // try L1 APC |
There was a problem hiding this comment.
Comment incorrectly refers to 'APC' but the code is using memcached. Should be 'try L1 memcached'.
| // try L1 APC | |
| // try L1 memcached |
| $encoded = MemCache::get($key); | ||
| $wasMemCacheHit = $encoded !== null; | ||
| if($wasMemCacheHit){ | ||
| Log::debug("CacheMiddleware:: MemcCache Hit"); |
There was a problem hiding this comment.
Typo in log message - 'MemcCache' should be 'Memcached'.
| Log::debug("CacheMiddleware:: MemcCache Hit"); | |
| Log::debug("CacheMiddleware:: Memcached Hit"); |
| }); | ||
|
|
||
|
|
||
| // backfill APC only if we actually have a value |
There was a problem hiding this comment.
Comment incorrectly refers to 'APC' but the code is using memcached. Should be 'backfill memcached only if we actually have a value'.
| // backfill APC only if we actually have a value | |
| // backfill memcached only if we actually have a value |
| } | ||
| $data = $this->decode($encoded); | ||
| } else { | ||
| // try L1 APC |
There was a problem hiding this comment.
Comment incorrectly refers to 'APC' but the code is using memcached. Should be 'try L1 memcached'.
| $encoded = MemCache::get($key); | ||
| $wasMemCacheHit = !is_null($encoded); | ||
| if($wasMemCacheHit){ | ||
| Log::debug("CacheMiddleware:: MemcCache Hit"); |
There was a problem hiding this comment.
Typo in log message - 'MemcCache' should be 'Memcached'.
| Log::debug("CacheMiddleware:: MemcCache Hit"); | |
| Log::debug("CacheMiddleware:: Memcached Hit"); |
| $status = $resp->getStatusCode(); | ||
| if($status === 200) | ||
| return $this->encode($resp->getData(true)); | ||
| // store at APC |
There was a problem hiding this comment.
Comment incorrectly refers to 'APC' but the code is using memcached. Should be 'store at memcached'.
| // store at APC | |
| // store at memcached |
| $response->headers->addCacheControlDirective('proxy-revalidate', true); | ||
| $response->headers->add([ | ||
| 'X-Cache-Result' => $wasHit ? 'HIT':'MISS', | ||
| 'X-Cache-Result' => $wasMemCacheHit ? 'HIT MemcCache' : ($wasHit ? 'HIT REDIS' : 'MISS'), |
There was a problem hiding this comment.
Typo in header value - 'MemcCache' should be 'Memcached'.
| 'X-Cache-Result' => $wasMemCacheHit ? 'HIT MemcCache' : ($wasHit ? 'HIT REDIS' : 'MISS'), | |
| 'X-Cache-Result' => $wasMemCacheHit ? 'HIT Memcached' : ($wasHit ? 'HIT REDIS' : 'MISS'), |
app/Services/Model/Imp/ProcessScheduleEntityLifeCycleEventService.php
Outdated
Show resolved
Hide resolved
…entService.php Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
smarcet
force-pushed
the
feature/l1-memcached-cache-middleware
branch
from
5855552 to
99cd13d
Compare
No description provided.