Fetching a cached Eloquent collection of 10 items times out

Multi tool use
I'm building a search functionality that returns large collections which are paginated using a LengthAwarePaginator. I'm trying to cache results using a key called $searchFilter_$query_$offsetPages for a single page of returned results (10 items). It goes into the cache just fine. However, it times out when I try to check using Cache::has($key) or fetch using Cache::get($key).
The same problem occurs in the browser as well as in artisan Tinker. Strangely, when I put a random set of 10 items into the cache in Tinker and fetch them back, everything works fine. I'm using Redis as the cache driver.
Here is my controller method:
public function search($filter, $query, $layout, Request $request) {
if($layout == "list-map") {
return view("list-map")->with(['filter' => $filter, 'query' => $query, 'layout' => 'list-map']);
} else {
$offsetPages = $request->input('page', 1) - 1;
$cacheKey = $filter . "_" . $query . "_" . $offsetPages;
if(Cache::has($cacheKey)) {
Log::info("fetching results from cache");
$data = Cache::get($cacheKey);
$totalCt = $data[0];
$results = $data[1];
} else {
$results = $this->getResults($filter, $query);
$totalCt = $results->count();
$results = $results->slice($offsetPages, $this->resultsPerPage);
Log::info("caching results");
Cache::put($cacheKey, [$totalCt, $results], 5);
}
$results = new LengthAwarePaginator($results,
$totalCt,
$this->resultsPerPage,
$request->input('page', 1),
['path' => LengthAwarePaginator::resolveCurrentPath()]
);
return view($layout)->with(['filter' => $filter, 'query' => $query, 'layout' => $layout, 'results' => $results]);
}
}
laravel redis
add a comment |
I'm building a search functionality that returns large collections which are paginated using a LengthAwarePaginator. I'm trying to cache results using a key called $searchFilter_$query_$offsetPages for a single page of returned results (10 items). It goes into the cache just fine. However, it times out when I try to check using Cache::has($key) or fetch using Cache::get($key).
The same problem occurs in the browser as well as in artisan Tinker. Strangely, when I put a random set of 10 items into the cache in Tinker and fetch them back, everything works fine. I'm using Redis as the cache driver.
Here is my controller method:
public function search($filter, $query, $layout, Request $request) {
if($layout == "list-map") {
return view("list-map")->with(['filter' => $filter, 'query' => $query, 'layout' => 'list-map']);
} else {
$offsetPages = $request->input('page', 1) - 1;
$cacheKey = $filter . "_" . $query . "_" . $offsetPages;
if(Cache::has($cacheKey)) {
Log::info("fetching results from cache");
$data = Cache::get($cacheKey);
$totalCt = $data[0];
$results = $data[1];
} else {
$results = $this->getResults($filter, $query);
$totalCt = $results->count();
$results = $results->slice($offsetPages, $this->resultsPerPage);
Log::info("caching results");
Cache::put($cacheKey, [$totalCt, $results], 5);
}
$results = new LengthAwarePaginator($results,
$totalCt,
$this->resultsPerPage,
$request->input('page', 1),
['path' => LengthAwarePaginator::resolveCurrentPath()]
);
return view($layout)->with(['filter' => $filter, 'query' => $query, 'layout' => $layout, 'results' => $results]);
}
}
laravel redis
What content has$filter
? Maybe build a hash of it to prevent using not allowed characters for a cache key.
– Namoshek
Dec 30 '18 at 21:02
$filter is either "doctor", "clinic", or "all"
– ThriftyNick
Dec 30 '18 at 21:03
And what about$query
?
– Namoshek
Dec 30 '18 at 21:04
Also $query does not contain spaces. They were replaced by underscores in a route closure and then redirected to this controller method.
– ThriftyNick
Dec 30 '18 at 21:10
dd($cacheKey) => doctor_substance_abuse_0
– ThriftyNick
Dec 30 '18 at 21:11
add a comment |
I'm building a search functionality that returns large collections which are paginated using a LengthAwarePaginator. I'm trying to cache results using a key called $searchFilter_$query_$offsetPages for a single page of returned results (10 items). It goes into the cache just fine. However, it times out when I try to check using Cache::has($key) or fetch using Cache::get($key).
The same problem occurs in the browser as well as in artisan Tinker. Strangely, when I put a random set of 10 items into the cache in Tinker and fetch them back, everything works fine. I'm using Redis as the cache driver.
Here is my controller method:
public function search($filter, $query, $layout, Request $request) {
if($layout == "list-map") {
return view("list-map")->with(['filter' => $filter, 'query' => $query, 'layout' => 'list-map']);
} else {
$offsetPages = $request->input('page', 1) - 1;
$cacheKey = $filter . "_" . $query . "_" . $offsetPages;
if(Cache::has($cacheKey)) {
Log::info("fetching results from cache");
$data = Cache::get($cacheKey);
$totalCt = $data[0];
$results = $data[1];
} else {
$results = $this->getResults($filter, $query);
$totalCt = $results->count();
$results = $results->slice($offsetPages, $this->resultsPerPage);
Log::info("caching results");
Cache::put($cacheKey, [$totalCt, $results], 5);
}
$results = new LengthAwarePaginator($results,
$totalCt,
$this->resultsPerPage,
$request->input('page', 1),
['path' => LengthAwarePaginator::resolveCurrentPath()]
);
return view($layout)->with(['filter' => $filter, 'query' => $query, 'layout' => $layout, 'results' => $results]);
}
}
laravel redis
I'm building a search functionality that returns large collections which are paginated using a LengthAwarePaginator. I'm trying to cache results using a key called $searchFilter_$query_$offsetPages for a single page of returned results (10 items). It goes into the cache just fine. However, it times out when I try to check using Cache::has($key) or fetch using Cache::get($key).
The same problem occurs in the browser as well as in artisan Tinker. Strangely, when I put a random set of 10 items into the cache in Tinker and fetch them back, everything works fine. I'm using Redis as the cache driver.
Here is my controller method:
public function search($filter, $query, $layout, Request $request) {
if($layout == "list-map") {
return view("list-map")->with(['filter' => $filter, 'query' => $query, 'layout' => 'list-map']);
} else {
$offsetPages = $request->input('page', 1) - 1;
$cacheKey = $filter . "_" . $query . "_" . $offsetPages;
if(Cache::has($cacheKey)) {
Log::info("fetching results from cache");
$data = Cache::get($cacheKey);
$totalCt = $data[0];
$results = $data[1];
} else {
$results = $this->getResults($filter, $query);
$totalCt = $results->count();
$results = $results->slice($offsetPages, $this->resultsPerPage);
Log::info("caching results");
Cache::put($cacheKey, [$totalCt, $results], 5);
}
$results = new LengthAwarePaginator($results,
$totalCt,
$this->resultsPerPage,
$request->input('page', 1),
['path' => LengthAwarePaginator::resolveCurrentPath()]
);
return view($layout)->with(['filter' => $filter, 'query' => $query, 'layout' => $layout, 'results' => $results]);
}
}
laravel redis
laravel redis
asked Dec 30 '18 at 20:24
ThriftyNickThriftyNick
207
207
What content has$filter
? Maybe build a hash of it to prevent using not allowed characters for a cache key.
– Namoshek
Dec 30 '18 at 21:02
$filter is either "doctor", "clinic", or "all"
– ThriftyNick
Dec 30 '18 at 21:03
And what about$query
?
– Namoshek
Dec 30 '18 at 21:04
Also $query does not contain spaces. They were replaced by underscores in a route closure and then redirected to this controller method.
– ThriftyNick
Dec 30 '18 at 21:10
dd($cacheKey) => doctor_substance_abuse_0
– ThriftyNick
Dec 30 '18 at 21:11
add a comment |
What content has$filter
? Maybe build a hash of it to prevent using not allowed characters for a cache key.
– Namoshek
Dec 30 '18 at 21:02
$filter is either "doctor", "clinic", or "all"
– ThriftyNick
Dec 30 '18 at 21:03
And what about$query
?
– Namoshek
Dec 30 '18 at 21:04
Also $query does not contain spaces. They were replaced by underscores in a route closure and then redirected to this controller method.
– ThriftyNick
Dec 30 '18 at 21:10
dd($cacheKey) => doctor_substance_abuse_0
– ThriftyNick
Dec 30 '18 at 21:11
What content has
$filter
? Maybe build a hash of it to prevent using not allowed characters for a cache key.– Namoshek
Dec 30 '18 at 21:02
What content has
$filter
? Maybe build a hash of it to prevent using not allowed characters for a cache key.– Namoshek
Dec 30 '18 at 21:02
$filter is either "doctor", "clinic", or "all"
– ThriftyNick
Dec 30 '18 at 21:03
$filter is either "doctor", "clinic", or "all"
– ThriftyNick
Dec 30 '18 at 21:03
And what about
$query
?– Namoshek
Dec 30 '18 at 21:04
And what about
$query
?– Namoshek
Dec 30 '18 at 21:04
Also $query does not contain spaces. They were replaced by underscores in a route closure and then redirected to this controller method.
– ThriftyNick
Dec 30 '18 at 21:10
Also $query does not contain spaces. They were replaced by underscores in a route closure and then redirected to this controller method.
– ThriftyNick
Dec 30 '18 at 21:10
dd($cacheKey) => doctor_substance_abuse_0
– ThriftyNick
Dec 30 '18 at 21:11
dd($cacheKey) => doctor_substance_abuse_0
– ThriftyNick
Dec 30 '18 at 21:11
add a comment |
2 Answers
2
active
oldest
votes
So, the issue was that many of the models in the collection returned from my getResults() method were obtained via relationship queries. When I would dd($results) on the single page of 10 results, I could see that there was a "relations" field on each model. Inside that array were thousands of recursively related models based on the relationship I originally queried. I was unable to find any information about an option to not eager load these related models. Instead I came up with a bit of a hacky workaround to fetch the models directly:
$results = $results->slice($offsetPages, $this->resultsPerPage);
//load models directly so they don't include related models.
$temp = new IlluminateDatabaseEloquentCollection;
foreach($results as $result) {
if(get_class($result) == "AppDoctor") {
$result = Doctor::find($result->id);
} else if(get_class($result == "AppOrganization")) {
$result = Organization::find($result->id);
}
$temp->push($result);
}
$results = $temp;
Log::info("caching results");
Cache::put($cacheKey, [$totalCt, $results], 5);
If anyone knows the best practice in this situation, please let me know. Thanks!
Edit:
I've found a better solution instead of the above workaround. If I query my relationships like this: $taxonomy->doctors()->get() rather than $taxonomy->doctors, it does not load in the huge recusive relations.
It is a rare use case, but maybe$model->newQueryWithoutRelationships()
is suitable for this task (when querying the results, to not load the relations at all). You can also unload a relation with$model->unsetRelation('doctor')
after they were loaded (which doesn't make much sense). I thinkt hat if you optimize your query, you don't need caching at all...
– Namoshek
Dec 31 '18 at 9:32
I'm not really sure how I would optimize it since it requires querying multiple tables, sometimes two levels deep via relationships to fetch meaningful search results. Thank you for the suggestions!
– ThriftyNick
Dec 31 '18 at 12:37
You could also build a query with joins manually, if you don't need multiple child rows of a relation. Maybe this is something for another question. ;)
– Namoshek
Dec 31 '18 at 12:41
That might be ideal, but I'm content with the current state of it. I need to move the project forward. Thank you for your insight.
– ThriftyNick
Dec 31 '18 at 16:24
add a comment |
I dont really see why your code doesn't work. The only potential problems I see are the cache keys, which could contain problematic characters, as well as the way you check for a cached value. As you are using Cache::has($key)
before Cache::get($key)
, you could end up with a race condition where the first call returns true
and the latter null
because the cached value timed out just between the two calls.
I tried to address both issues in the following snippet:
public function search($filter, $query, $layout, Request $request)
{
if($layout == "list-map") {
return view("list-map")->with(['filter' => $filter, 'query' => $query, 'layout' => 'list-map']);
} else {
$offsetPages = $request->input('page', 1) - 1;
$cacheKey = md5("{$filter}_{$query}_{$offsetPages}");
$duration = 5; // todo: make this configurable or a constant
[$totalCount, $results] = Cache::remember($cacheKey, $duration, function () use ($filter, $query) {
$results = $this->getResults($filter, $query);
$totalCount = $results->count();
$filteredResults = $results->slice($offsetPages, $this->resultsPerPage);
return [$totalCount, $filteredResults];
});
$results = new LengthAwarePaginator($results,
$totalCount,
$this->resultsPerPage,
$request->input('page', 1),
['path' => LengthAwarePaginator::resolveCurrentPath()]
);
return view($layout)->with(compact('filter', 'query', 'layout', 'results'));
}
}
The inbuilt function Cache::remember()
doesn't use Cache::has()
under the hood. Instead, it will simply call Cache::get()
. As this function will return null
as default if no cache was hit, the function can easily determine if it has to execute the closure or not.
I also wrapped the $cacheKey
in md5()
, which gives a consistently valid key.
Looking at the following part of your code
$results = $this->getResults($filter, $query);
$totalCount = $results->count();
$filteredResults = $results->slice($offsetPages, $this->resultsPerPage);
I am quite sure the whole search could be improved (independently of the caching). Because it seems you are loading all results for a specific search into memory, even if you throw away most parts of it. There is certainly a better way to do this.
Thank you for your feedback. I'll see if hashing the key makes any difference. I did try using the remember function at first, but I think I ran into an issue calling $this->getResults() since I believe $this would be out of scope inside the closure. I tried setting it to $self before and passing it in with use, but I think there were still issues with that.
– ThriftyNick
Dec 30 '18 at 22:44
I just tried changing the condition to if(!is_null($data = Cache::get($cacheKey))) instead of using has followed by get. The problem still remains.
– ThriftyNick
Dec 30 '18 at 22:46
hashing the key did not make a difference either.
– ThriftyNick
Dec 30 '18 at 22:51
Ah, it's something to do with the collection I'm returning. When I simply take the first 500 records, slice, and cache, it works fine. I'll continue to investigate what I'm returning from $this->getResults(). I'll let you know what I find out. Thanks.
– ThriftyNick
Dec 30 '18 at 23:15
The problem is that I'm getting most of the models in the collection via relationships and it's loading in everything related to each model. So, when I dd() and inspect a model, there is a "relations" array that holds all related models. Is there a way to not include these related models?
– ThriftyNick
Dec 31 '18 at 0:06
add a comment |
Your Answer
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "1"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: true,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: 10,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53981151%2ffetching-a-cached-eloquent-collection-of-10-items-times-out%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
So, the issue was that many of the models in the collection returned from my getResults() method were obtained via relationship queries. When I would dd($results) on the single page of 10 results, I could see that there was a "relations" field on each model. Inside that array were thousands of recursively related models based on the relationship I originally queried. I was unable to find any information about an option to not eager load these related models. Instead I came up with a bit of a hacky workaround to fetch the models directly:
$results = $results->slice($offsetPages, $this->resultsPerPage);
//load models directly so they don't include related models.
$temp = new IlluminateDatabaseEloquentCollection;
foreach($results as $result) {
if(get_class($result) == "AppDoctor") {
$result = Doctor::find($result->id);
} else if(get_class($result == "AppOrganization")) {
$result = Organization::find($result->id);
}
$temp->push($result);
}
$results = $temp;
Log::info("caching results");
Cache::put($cacheKey, [$totalCt, $results], 5);
If anyone knows the best practice in this situation, please let me know. Thanks!
Edit:
I've found a better solution instead of the above workaround. If I query my relationships like this: $taxonomy->doctors()->get() rather than $taxonomy->doctors, it does not load in the huge recusive relations.
It is a rare use case, but maybe$model->newQueryWithoutRelationships()
is suitable for this task (when querying the results, to not load the relations at all). You can also unload a relation with$model->unsetRelation('doctor')
after they were loaded (which doesn't make much sense). I thinkt hat if you optimize your query, you don't need caching at all...
– Namoshek
Dec 31 '18 at 9:32
I'm not really sure how I would optimize it since it requires querying multiple tables, sometimes two levels deep via relationships to fetch meaningful search results. Thank you for the suggestions!
– ThriftyNick
Dec 31 '18 at 12:37
You could also build a query with joins manually, if you don't need multiple child rows of a relation. Maybe this is something for another question. ;)
– Namoshek
Dec 31 '18 at 12:41
That might be ideal, but I'm content with the current state of it. I need to move the project forward. Thank you for your insight.
– ThriftyNick
Dec 31 '18 at 16:24
add a comment |
So, the issue was that many of the models in the collection returned from my getResults() method were obtained via relationship queries. When I would dd($results) on the single page of 10 results, I could see that there was a "relations" field on each model. Inside that array were thousands of recursively related models based on the relationship I originally queried. I was unable to find any information about an option to not eager load these related models. Instead I came up with a bit of a hacky workaround to fetch the models directly:
$results = $results->slice($offsetPages, $this->resultsPerPage);
//load models directly so they don't include related models.
$temp = new IlluminateDatabaseEloquentCollection;
foreach($results as $result) {
if(get_class($result) == "AppDoctor") {
$result = Doctor::find($result->id);
} else if(get_class($result == "AppOrganization")) {
$result = Organization::find($result->id);
}
$temp->push($result);
}
$results = $temp;
Log::info("caching results");
Cache::put($cacheKey, [$totalCt, $results], 5);
If anyone knows the best practice in this situation, please let me know. Thanks!
Edit:
I've found a better solution instead of the above workaround. If I query my relationships like this: $taxonomy->doctors()->get() rather than $taxonomy->doctors, it does not load in the huge recusive relations.
It is a rare use case, but maybe$model->newQueryWithoutRelationships()
is suitable for this task (when querying the results, to not load the relations at all). You can also unload a relation with$model->unsetRelation('doctor')
after they were loaded (which doesn't make much sense). I thinkt hat if you optimize your query, you don't need caching at all...
– Namoshek
Dec 31 '18 at 9:32
I'm not really sure how I would optimize it since it requires querying multiple tables, sometimes two levels deep via relationships to fetch meaningful search results. Thank you for the suggestions!
– ThriftyNick
Dec 31 '18 at 12:37
You could also build a query with joins manually, if you don't need multiple child rows of a relation. Maybe this is something for another question. ;)
– Namoshek
Dec 31 '18 at 12:41
That might be ideal, but I'm content with the current state of it. I need to move the project forward. Thank you for your insight.
– ThriftyNick
Dec 31 '18 at 16:24
add a comment |
So, the issue was that many of the models in the collection returned from my getResults() method were obtained via relationship queries. When I would dd($results) on the single page of 10 results, I could see that there was a "relations" field on each model. Inside that array were thousands of recursively related models based on the relationship I originally queried. I was unable to find any information about an option to not eager load these related models. Instead I came up with a bit of a hacky workaround to fetch the models directly:
$results = $results->slice($offsetPages, $this->resultsPerPage);
//load models directly so they don't include related models.
$temp = new IlluminateDatabaseEloquentCollection;
foreach($results as $result) {
if(get_class($result) == "AppDoctor") {
$result = Doctor::find($result->id);
} else if(get_class($result == "AppOrganization")) {
$result = Organization::find($result->id);
}
$temp->push($result);
}
$results = $temp;
Log::info("caching results");
Cache::put($cacheKey, [$totalCt, $results], 5);
If anyone knows the best practice in this situation, please let me know. Thanks!
Edit:
I've found a better solution instead of the above workaround. If I query my relationships like this: $taxonomy->doctors()->get() rather than $taxonomy->doctors, it does not load in the huge recusive relations.
So, the issue was that many of the models in the collection returned from my getResults() method were obtained via relationship queries. When I would dd($results) on the single page of 10 results, I could see that there was a "relations" field on each model. Inside that array were thousands of recursively related models based on the relationship I originally queried. I was unable to find any information about an option to not eager load these related models. Instead I came up with a bit of a hacky workaround to fetch the models directly:
$results = $results->slice($offsetPages, $this->resultsPerPage);
//load models directly so they don't include related models.
$temp = new IlluminateDatabaseEloquentCollection;
foreach($results as $result) {
if(get_class($result) == "AppDoctor") {
$result = Doctor::find($result->id);
} else if(get_class($result == "AppOrganization")) {
$result = Organization::find($result->id);
}
$temp->push($result);
}
$results = $temp;
Log::info("caching results");
Cache::put($cacheKey, [$totalCt, $results], 5);
If anyone knows the best practice in this situation, please let me know. Thanks!
Edit:
I've found a better solution instead of the above workaround. If I query my relationships like this: $taxonomy->doctors()->get() rather than $taxonomy->doctors, it does not load in the huge recusive relations.
edited Dec 31 '18 at 2:08
answered Dec 31 '18 at 1:37
ThriftyNickThriftyNick
207
207
It is a rare use case, but maybe$model->newQueryWithoutRelationships()
is suitable for this task (when querying the results, to not load the relations at all). You can also unload a relation with$model->unsetRelation('doctor')
after they were loaded (which doesn't make much sense). I thinkt hat if you optimize your query, you don't need caching at all...
– Namoshek
Dec 31 '18 at 9:32
I'm not really sure how I would optimize it since it requires querying multiple tables, sometimes two levels deep via relationships to fetch meaningful search results. Thank you for the suggestions!
– ThriftyNick
Dec 31 '18 at 12:37
You could also build a query with joins manually, if you don't need multiple child rows of a relation. Maybe this is something for another question. ;)
– Namoshek
Dec 31 '18 at 12:41
That might be ideal, but I'm content with the current state of it. I need to move the project forward. Thank you for your insight.
– ThriftyNick
Dec 31 '18 at 16:24
add a comment |
It is a rare use case, but maybe$model->newQueryWithoutRelationships()
is suitable for this task (when querying the results, to not load the relations at all). You can also unload a relation with$model->unsetRelation('doctor')
after they were loaded (which doesn't make much sense). I thinkt hat if you optimize your query, you don't need caching at all...
– Namoshek
Dec 31 '18 at 9:32
I'm not really sure how I would optimize it since it requires querying multiple tables, sometimes two levels deep via relationships to fetch meaningful search results. Thank you for the suggestions!
– ThriftyNick
Dec 31 '18 at 12:37
You could also build a query with joins manually, if you don't need multiple child rows of a relation. Maybe this is something for another question. ;)
– Namoshek
Dec 31 '18 at 12:41
That might be ideal, but I'm content with the current state of it. I need to move the project forward. Thank you for your insight.
– ThriftyNick
Dec 31 '18 at 16:24
It is a rare use case, but maybe
$model->newQueryWithoutRelationships()
is suitable for this task (when querying the results, to not load the relations at all). You can also unload a relation with $model->unsetRelation('doctor')
after they were loaded (which doesn't make much sense). I thinkt hat if you optimize your query, you don't need caching at all...– Namoshek
Dec 31 '18 at 9:32
It is a rare use case, but maybe
$model->newQueryWithoutRelationships()
is suitable for this task (when querying the results, to not load the relations at all). You can also unload a relation with $model->unsetRelation('doctor')
after they were loaded (which doesn't make much sense). I thinkt hat if you optimize your query, you don't need caching at all...– Namoshek
Dec 31 '18 at 9:32
I'm not really sure how I would optimize it since it requires querying multiple tables, sometimes two levels deep via relationships to fetch meaningful search results. Thank you for the suggestions!
– ThriftyNick
Dec 31 '18 at 12:37
I'm not really sure how I would optimize it since it requires querying multiple tables, sometimes two levels deep via relationships to fetch meaningful search results. Thank you for the suggestions!
– ThriftyNick
Dec 31 '18 at 12:37
You could also build a query with joins manually, if you don't need multiple child rows of a relation. Maybe this is something for another question. ;)
– Namoshek
Dec 31 '18 at 12:41
You could also build a query with joins manually, if you don't need multiple child rows of a relation. Maybe this is something for another question. ;)
– Namoshek
Dec 31 '18 at 12:41
That might be ideal, but I'm content with the current state of it. I need to move the project forward. Thank you for your insight.
– ThriftyNick
Dec 31 '18 at 16:24
That might be ideal, but I'm content with the current state of it. I need to move the project forward. Thank you for your insight.
– ThriftyNick
Dec 31 '18 at 16:24
add a comment |
I dont really see why your code doesn't work. The only potential problems I see are the cache keys, which could contain problematic characters, as well as the way you check for a cached value. As you are using Cache::has($key)
before Cache::get($key)
, you could end up with a race condition where the first call returns true
and the latter null
because the cached value timed out just between the two calls.
I tried to address both issues in the following snippet:
public function search($filter, $query, $layout, Request $request)
{
if($layout == "list-map") {
return view("list-map")->with(['filter' => $filter, 'query' => $query, 'layout' => 'list-map']);
} else {
$offsetPages = $request->input('page', 1) - 1;
$cacheKey = md5("{$filter}_{$query}_{$offsetPages}");
$duration = 5; // todo: make this configurable or a constant
[$totalCount, $results] = Cache::remember($cacheKey, $duration, function () use ($filter, $query) {
$results = $this->getResults($filter, $query);
$totalCount = $results->count();
$filteredResults = $results->slice($offsetPages, $this->resultsPerPage);
return [$totalCount, $filteredResults];
});
$results = new LengthAwarePaginator($results,
$totalCount,
$this->resultsPerPage,
$request->input('page', 1),
['path' => LengthAwarePaginator::resolveCurrentPath()]
);
return view($layout)->with(compact('filter', 'query', 'layout', 'results'));
}
}
The inbuilt function Cache::remember()
doesn't use Cache::has()
under the hood. Instead, it will simply call Cache::get()
. As this function will return null
as default if no cache was hit, the function can easily determine if it has to execute the closure or not.
I also wrapped the $cacheKey
in md5()
, which gives a consistently valid key.
Looking at the following part of your code
$results = $this->getResults($filter, $query);
$totalCount = $results->count();
$filteredResults = $results->slice($offsetPages, $this->resultsPerPage);
I am quite sure the whole search could be improved (independently of the caching). Because it seems you are loading all results for a specific search into memory, even if you throw away most parts of it. There is certainly a better way to do this.
Thank you for your feedback. I'll see if hashing the key makes any difference. I did try using the remember function at first, but I think I ran into an issue calling $this->getResults() since I believe $this would be out of scope inside the closure. I tried setting it to $self before and passing it in with use, but I think there were still issues with that.
– ThriftyNick
Dec 30 '18 at 22:44
I just tried changing the condition to if(!is_null($data = Cache::get($cacheKey))) instead of using has followed by get. The problem still remains.
– ThriftyNick
Dec 30 '18 at 22:46
hashing the key did not make a difference either.
– ThriftyNick
Dec 30 '18 at 22:51
Ah, it's something to do with the collection I'm returning. When I simply take the first 500 records, slice, and cache, it works fine. I'll continue to investigate what I'm returning from $this->getResults(). I'll let you know what I find out. Thanks.
– ThriftyNick
Dec 30 '18 at 23:15
The problem is that I'm getting most of the models in the collection via relationships and it's loading in everything related to each model. So, when I dd() and inspect a model, there is a "relations" array that holds all related models. Is there a way to not include these related models?
– ThriftyNick
Dec 31 '18 at 0:06
add a comment |
I dont really see why your code doesn't work. The only potential problems I see are the cache keys, which could contain problematic characters, as well as the way you check for a cached value. As you are using Cache::has($key)
before Cache::get($key)
, you could end up with a race condition where the first call returns true
and the latter null
because the cached value timed out just between the two calls.
I tried to address both issues in the following snippet:
public function search($filter, $query, $layout, Request $request)
{
if($layout == "list-map") {
return view("list-map")->with(['filter' => $filter, 'query' => $query, 'layout' => 'list-map']);
} else {
$offsetPages = $request->input('page', 1) - 1;
$cacheKey = md5("{$filter}_{$query}_{$offsetPages}");
$duration = 5; // todo: make this configurable or a constant
[$totalCount, $results] = Cache::remember($cacheKey, $duration, function () use ($filter, $query) {
$results = $this->getResults($filter, $query);
$totalCount = $results->count();
$filteredResults = $results->slice($offsetPages, $this->resultsPerPage);
return [$totalCount, $filteredResults];
});
$results = new LengthAwarePaginator($results,
$totalCount,
$this->resultsPerPage,
$request->input('page', 1),
['path' => LengthAwarePaginator::resolveCurrentPath()]
);
return view($layout)->with(compact('filter', 'query', 'layout', 'results'));
}
}
The inbuilt function Cache::remember()
doesn't use Cache::has()
under the hood. Instead, it will simply call Cache::get()
. As this function will return null
as default if no cache was hit, the function can easily determine if it has to execute the closure or not.
I also wrapped the $cacheKey
in md5()
, which gives a consistently valid key.
Looking at the following part of your code
$results = $this->getResults($filter, $query);
$totalCount = $results->count();
$filteredResults = $results->slice($offsetPages, $this->resultsPerPage);
I am quite sure the whole search could be improved (independently of the caching). Because it seems you are loading all results for a specific search into memory, even if you throw away most parts of it. There is certainly a better way to do this.
Thank you for your feedback. I'll see if hashing the key makes any difference. I did try using the remember function at first, but I think I ran into an issue calling $this->getResults() since I believe $this would be out of scope inside the closure. I tried setting it to $self before and passing it in with use, but I think there were still issues with that.
– ThriftyNick
Dec 30 '18 at 22:44
I just tried changing the condition to if(!is_null($data = Cache::get($cacheKey))) instead of using has followed by get. The problem still remains.
– ThriftyNick
Dec 30 '18 at 22:46
hashing the key did not make a difference either.
– ThriftyNick
Dec 30 '18 at 22:51
Ah, it's something to do with the collection I'm returning. When I simply take the first 500 records, slice, and cache, it works fine. I'll continue to investigate what I'm returning from $this->getResults(). I'll let you know what I find out. Thanks.
– ThriftyNick
Dec 30 '18 at 23:15
The problem is that I'm getting most of the models in the collection via relationships and it's loading in everything related to each model. So, when I dd() and inspect a model, there is a "relations" array that holds all related models. Is there a way to not include these related models?
– ThriftyNick
Dec 31 '18 at 0:06
add a comment |
I dont really see why your code doesn't work. The only potential problems I see are the cache keys, which could contain problematic characters, as well as the way you check for a cached value. As you are using Cache::has($key)
before Cache::get($key)
, you could end up with a race condition where the first call returns true
and the latter null
because the cached value timed out just between the two calls.
I tried to address both issues in the following snippet:
public function search($filter, $query, $layout, Request $request)
{
if($layout == "list-map") {
return view("list-map")->with(['filter' => $filter, 'query' => $query, 'layout' => 'list-map']);
} else {
$offsetPages = $request->input('page', 1) - 1;
$cacheKey = md5("{$filter}_{$query}_{$offsetPages}");
$duration = 5; // todo: make this configurable or a constant
[$totalCount, $results] = Cache::remember($cacheKey, $duration, function () use ($filter, $query) {
$results = $this->getResults($filter, $query);
$totalCount = $results->count();
$filteredResults = $results->slice($offsetPages, $this->resultsPerPage);
return [$totalCount, $filteredResults];
});
$results = new LengthAwarePaginator($results,
$totalCount,
$this->resultsPerPage,
$request->input('page', 1),
['path' => LengthAwarePaginator::resolveCurrentPath()]
);
return view($layout)->with(compact('filter', 'query', 'layout', 'results'));
}
}
The inbuilt function Cache::remember()
doesn't use Cache::has()
under the hood. Instead, it will simply call Cache::get()
. As this function will return null
as default if no cache was hit, the function can easily determine if it has to execute the closure or not.
I also wrapped the $cacheKey
in md5()
, which gives a consistently valid key.
Looking at the following part of your code
$results = $this->getResults($filter, $query);
$totalCount = $results->count();
$filteredResults = $results->slice($offsetPages, $this->resultsPerPage);
I am quite sure the whole search could be improved (independently of the caching). Because it seems you are loading all results for a specific search into memory, even if you throw away most parts of it. There is certainly a better way to do this.
I dont really see why your code doesn't work. The only potential problems I see are the cache keys, which could contain problematic characters, as well as the way you check for a cached value. As you are using Cache::has($key)
before Cache::get($key)
, you could end up with a race condition where the first call returns true
and the latter null
because the cached value timed out just between the two calls.
I tried to address both issues in the following snippet:
public function search($filter, $query, $layout, Request $request)
{
if($layout == "list-map") {
return view("list-map")->with(['filter' => $filter, 'query' => $query, 'layout' => 'list-map']);
} else {
$offsetPages = $request->input('page', 1) - 1;
$cacheKey = md5("{$filter}_{$query}_{$offsetPages}");
$duration = 5; // todo: make this configurable or a constant
[$totalCount, $results] = Cache::remember($cacheKey, $duration, function () use ($filter, $query) {
$results = $this->getResults($filter, $query);
$totalCount = $results->count();
$filteredResults = $results->slice($offsetPages, $this->resultsPerPage);
return [$totalCount, $filteredResults];
});
$results = new LengthAwarePaginator($results,
$totalCount,
$this->resultsPerPage,
$request->input('page', 1),
['path' => LengthAwarePaginator::resolveCurrentPath()]
);
return view($layout)->with(compact('filter', 'query', 'layout', 'results'));
}
}
The inbuilt function Cache::remember()
doesn't use Cache::has()
under the hood. Instead, it will simply call Cache::get()
. As this function will return null
as default if no cache was hit, the function can easily determine if it has to execute the closure or not.
I also wrapped the $cacheKey
in md5()
, which gives a consistently valid key.
Looking at the following part of your code
$results = $this->getResults($filter, $query);
$totalCount = $results->count();
$filteredResults = $results->slice($offsetPages, $this->resultsPerPage);
I am quite sure the whole search could be improved (independently of the caching). Because it seems you are loading all results for a specific search into memory, even if you throw away most parts of it. There is certainly a better way to do this.
answered Dec 30 '18 at 21:35
NamoshekNamoshek
2,9812819
2,9812819
Thank you for your feedback. I'll see if hashing the key makes any difference. I did try using the remember function at first, but I think I ran into an issue calling $this->getResults() since I believe $this would be out of scope inside the closure. I tried setting it to $self before and passing it in with use, but I think there were still issues with that.
– ThriftyNick
Dec 30 '18 at 22:44
I just tried changing the condition to if(!is_null($data = Cache::get($cacheKey))) instead of using has followed by get. The problem still remains.
– ThriftyNick
Dec 30 '18 at 22:46
hashing the key did not make a difference either.
– ThriftyNick
Dec 30 '18 at 22:51
Ah, it's something to do with the collection I'm returning. When I simply take the first 500 records, slice, and cache, it works fine. I'll continue to investigate what I'm returning from $this->getResults(). I'll let you know what I find out. Thanks.
– ThriftyNick
Dec 30 '18 at 23:15
The problem is that I'm getting most of the models in the collection via relationships and it's loading in everything related to each model. So, when I dd() and inspect a model, there is a "relations" array that holds all related models. Is there a way to not include these related models?
– ThriftyNick
Dec 31 '18 at 0:06
add a comment |
Thank you for your feedback. I'll see if hashing the key makes any difference. I did try using the remember function at first, but I think I ran into an issue calling $this->getResults() since I believe $this would be out of scope inside the closure. I tried setting it to $self before and passing it in with use, but I think there were still issues with that.
– ThriftyNick
Dec 30 '18 at 22:44
I just tried changing the condition to if(!is_null($data = Cache::get($cacheKey))) instead of using has followed by get. The problem still remains.
– ThriftyNick
Dec 30 '18 at 22:46
hashing the key did not make a difference either.
– ThriftyNick
Dec 30 '18 at 22:51
Ah, it's something to do with the collection I'm returning. When I simply take the first 500 records, slice, and cache, it works fine. I'll continue to investigate what I'm returning from $this->getResults(). I'll let you know what I find out. Thanks.
– ThriftyNick
Dec 30 '18 at 23:15
The problem is that I'm getting most of the models in the collection via relationships and it's loading in everything related to each model. So, when I dd() and inspect a model, there is a "relations" array that holds all related models. Is there a way to not include these related models?
– ThriftyNick
Dec 31 '18 at 0:06
Thank you for your feedback. I'll see if hashing the key makes any difference. I did try using the remember function at first, but I think I ran into an issue calling $this->getResults() since I believe $this would be out of scope inside the closure. I tried setting it to $self before and passing it in with use, but I think there were still issues with that.
– ThriftyNick
Dec 30 '18 at 22:44
Thank you for your feedback. I'll see if hashing the key makes any difference. I did try using the remember function at first, but I think I ran into an issue calling $this->getResults() since I believe $this would be out of scope inside the closure. I tried setting it to $self before and passing it in with use, but I think there were still issues with that.
– ThriftyNick
Dec 30 '18 at 22:44
I just tried changing the condition to if(!is_null($data = Cache::get($cacheKey))) instead of using has followed by get. The problem still remains.
– ThriftyNick
Dec 30 '18 at 22:46
I just tried changing the condition to if(!is_null($data = Cache::get($cacheKey))) instead of using has followed by get. The problem still remains.
– ThriftyNick
Dec 30 '18 at 22:46
hashing the key did not make a difference either.
– ThriftyNick
Dec 30 '18 at 22:51
hashing the key did not make a difference either.
– ThriftyNick
Dec 30 '18 at 22:51
Ah, it's something to do with the collection I'm returning. When I simply take the first 500 records, slice, and cache, it works fine. I'll continue to investigate what I'm returning from $this->getResults(). I'll let you know what I find out. Thanks.
– ThriftyNick
Dec 30 '18 at 23:15
Ah, it's something to do with the collection I'm returning. When I simply take the first 500 records, slice, and cache, it works fine. I'll continue to investigate what I'm returning from $this->getResults(). I'll let you know what I find out. Thanks.
– ThriftyNick
Dec 30 '18 at 23:15
The problem is that I'm getting most of the models in the collection via relationships and it's loading in everything related to each model. So, when I dd() and inspect a model, there is a "relations" array that holds all related models. Is there a way to not include these related models?
– ThriftyNick
Dec 31 '18 at 0:06
The problem is that I'm getting most of the models in the collection via relationships and it's loading in everything related to each model. So, when I dd() and inspect a model, there is a "relations" array that holds all related models. Is there a way to not include these related models?
– ThriftyNick
Dec 31 '18 at 0:06
add a comment |
Thanks for contributing an answer to Stack Overflow!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53981151%2ffetching-a-cached-eloquent-collection-of-10-items-times-out%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
B9u2gzjkiY ZOp D5hRY8,gV,VUtkYfUpxIYUI,B 7zK9c3a8lueyxXiU8,YwrxPo1 nNJRb9fMIT,q1hjHLvrdJf,1vUpn8sh
What content has
$filter
? Maybe build a hash of it to prevent using not allowed characters for a cache key.– Namoshek
Dec 30 '18 at 21:02
$filter is either "doctor", "clinic", or "all"
– ThriftyNick
Dec 30 '18 at 21:03
And what about
$query
?– Namoshek
Dec 30 '18 at 21:04
Also $query does not contain spaces. They were replaced by underscores in a route closure and then redirected to this controller method.
– ThriftyNick
Dec 30 '18 at 21:10
dd($cacheKey) => doctor_substance_abuse_0
– ThriftyNick
Dec 30 '18 at 21:11