Skip to content

Commit

Permalink
Fixed eager-loading performance issue in sockets, though it is still …
Browse files Browse the repository at this point in the history
…commented out until we can test, and made wrenceh and socket models fillable to make testing the data-options easier (#130
  • Loading branch information
kchapple committed Jul 24, 2020
1 parent ba6a84c commit 57631aa
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public function up()
$table->integer( 'wrench_id' );
$table->string( 'socket_value', 1024 );
$table->string('socket_label', 1024);
$table->boolean( 'is_default_socket' );
$table->boolean( 'is_default_socket' )->default(0);
$table->integer( 'socketsource_id');
$table->timestamp('created_at')->useCurrent();
$table->timestamp('updated_at')->useCurrent();
Expand Down
2 changes: 2 additions & 0 deletions src/CareSet/Zermelo/Models/Socket.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ class Socket extends AbstractZermeloModel
{
protected $table = 'socket';

protected $fillable = ['wrench_id', 'socket_value', 'socket_label', 'is_default_socket', 'socketsource_id'];

public function wrench()
{
return $this->belongsTo( Wrench::class );
Expand Down
7 changes: 7 additions & 0 deletions src/CareSet/Zermelo/Models/Wrench.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ class Wrench extends AbstractZermeloModel
{
protected $table = 'wrench';

protected $fillable = ['wrench_lookup_string', 'wrench_label'];

/*
* Eager-load the sockets
*/
protected $with = ['sockets'];

public function sockets()
{
return $this->hasMany(Socket::class )->orderBy('socket_label');
Expand Down
61 changes: 32 additions & 29 deletions src/CareSet/Zermelo/Services/SocketService.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,20 +52,20 @@ public function setSocketsFromApiInput( $input )
}

/*
Get all of the sockets for a given wrenchString aand
return then as an associative array...
Get all of the sockets for a given wrenchString aand
return then as an associative array...
*/
public function fetchAllSocketsForWrenchKey( $key )
{
$wrench = Wrench::where( 'wrench_lookup_string', $key )->first();
if ( $wrench !== null ) {

//I know their is an Eloquent way to do this.. not work the bother...
//I know their is an Eloquent way to do this.. not work the bother...
$sockets = Socket::where('wrench_id', $wrench->id)->get();

$socket_list_to_return = [];

//lets build a simple flat list with the expected contents...
//lets build a simple flat list with the expected contents...
foreach($sockets as $this_socket){
$socket_list_to_return[$this_socket->id] = [
'socket_label' => $this_socket->socket_label,
Expand Down Expand Up @@ -129,51 +129,54 @@ public function fetchSocketForWrenchKey( $key )
/**
* Check the default socket in sockets table to make sure that
* each wrench as one, and only one, default socket
* TODO, make a whole logging and warning system : https://github.com/CareSet/Zermelo/issues/107
*
* TODO, make a whole logging and warning system : https://github.com/CareSet/Zermelo/issues/107
*
*/
public static function checkIsDefaultSocket()
{

//Note with lots of sockets 80k and a few hundred wrenches...
//the code below causes the entire system to crash.
//the code calls the socket objects many many times.
//This needs to be converted into a single raw SQL statement. The following SQL statement should have no results:
//the code below causes the entire system to crash.
//the code calls the socket objects many many times.
//This needs to be converted into a single raw SQL statement. The following SQL statement should have no results:

/*
SELECT wrench_id
FROM socket
SELECT wrench_id
FROM socket
GROUP BY `wrench_id`
HAVING SUM(is_default_socket) > 1
UNION
SELECT wrench_id
UNION
SELECT wrench_id
FROM socket
GROUP BY wrench_id
HAVING SUM(is_default_socket) = 0
*/

//until then lets protect ourselves from this massive performance hit not running the code below
return(true);
// We have adjusted the below code to fetch wrenches, and Eager-Load (join) the sockets along
// with them, so performance should no longer be an issue. This hasn't been fully tested on large data
// until then lets protect ourselves from this massive performance hit not running the code below
return(true);




// Fetch all the sockets
$sockets = Socket::all();
// Get all the wrenches with sockets eager-loaded (joined)
$wrenches = Wrench::all();

// Count the default sockets for each
$default_hist = [];
foreach ($sockets as $socket) {

if(is_object($socket->wrench)){ //to account for cases where we have sockets without wrenches
if (!isset($default_hist[$socket->wrench->wrench_label])) {
$default_hist[$socket->wrench->wrench_label] = 0;
}

if ($socket->is_default_socket == 1) {
$default_hist[$socket->wrench->wrench_label]++;
}
}
foreach ($wrenches as $wrench) {
foreach ($wrench->sockets as $socket) {
if (is_object($socket->wrench)) { //to account for cases where we have sockets without wrenches
if (!isset($default_hist[$socket->wrench->wrench_label])) {
$default_hist[$socket->wrench->wrench_label] = 0;
}

if ($socket->is_default_socket == 1) {
$default_hist[$socket->wrench->wrench_label]++;
}
}
}
}

$message = "";
Expand Down

0 comments on commit 57631aa

Please sign in to comment.