From aa3f68f520ee10ad05f600a174628451221b5590 Mon Sep 17 00:00:00 2001 From: Reggie Riser Date: Tue, 9 Feb 2021 16:21:54 +0000 Subject: [PATCH 1/5] Using EagerPreload to optimize fetch-mto-updates --- pkg/models/order.go | 4 ++-- pkg/services/move_task_order/move_task_order_fetcher.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/models/order.go b/pkg/models/order.go index dacb47e0e5e..73991b0758e 100644 --- a/pkg/models/order.go +++ b/pkg/models/order.go @@ -44,7 +44,7 @@ type Order struct { HasDependents bool `json:"has_dependents" db:"has_dependents"` SpouseHasProGear bool `json:"spouse_has_pro_gear" db:"spouse_has_pro_gear"` NewDutyStationID uuid.UUID `json:"new_duty_station_id" db:"new_duty_station_id"` - NewDutyStation DutyStation `belongs_to:"duty_stations"` + NewDutyStation DutyStation `belongs_to:"duty_stations" fk_id:"new_duty_station_id"` UploadedOrders Document `belongs_to:"documents"` UploadedOrdersID uuid.UUID `json:"uploaded_orders_id" db:"uploaded_orders_id"` OrdersNumber *string `json:"orders_number" db:"orders_number"` @@ -56,7 +56,7 @@ type Order struct { Grade *string `json:"grade" db:"grade"` Entitlement *Entitlement `belongs_to:"entitlements"` EntitlementID *uuid.UUID `json:"entitlement_id" db:"entitlement_id"` - OriginDutyStation *DutyStation `belongs_to:"duty_stations"` + OriginDutyStation *DutyStation `belongs_to:"duty_stations" fk_id:"origin_duty_station_id"` OriginDutyStationID *uuid.UUID `json:"origin_duty_station_id" db:"origin_duty_station_id"` } diff --git a/pkg/services/move_task_order/move_task_order_fetcher.go b/pkg/services/move_task_order/move_task_order_fetcher.go index cfaad87cd34..9de808ed90b 100644 --- a/pkg/services/move_task_order/move_task_order_fetcher.go +++ b/pkg/services/move_task_order/move_task_order_fetcher.go @@ -41,7 +41,7 @@ func (f moveTaskOrderFetcher) ListMoveTaskOrders(moveOrderID uuid.UUID, searchPa func (f moveTaskOrderFetcher) ListAllMoveTaskOrders(searchParams *services.ListMoveTaskOrderParams) (models.Moves, error) { var moveTaskOrders models.Moves var err error - query := f.db.Q().Eager( + query := f.db.Q().EagerPreload( "PaymentRequests.PaymentServiceItems.PaymentServiceItemParams.ServiceItemParamKey", "MTOServiceItems.ReService", "MTOServiceItems.Dimensions", From 6e2df412371ddee6a1ad16484692ea828aa4bb3f Mon Sep 17 00:00:00 2001 From: Reggie Riser Date: Tue, 9 Feb 2021 16:25:44 +0000 Subject: [PATCH 2/5] Using EagerPreload to optimize TOO queue --- pkg/services/move_order/move_order_fetcher.go | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/pkg/services/move_order/move_order_fetcher.go b/pkg/services/move_order/move_order_fetcher.go index 8bdf1b03296..3f8fc0c1660 100644 --- a/pkg/services/move_order/move_order_fetcher.go +++ b/pkg/services/move_order/move_order_fetcher.go @@ -57,10 +57,11 @@ func (f moveOrderFetcher) ListMoveOrders(officeUserID uuid.UUID, params *service // Adding to an array so we can iterate over them and apply the filters after the query structure is set below options := []QueryOption{branchQuery, locatorQuery, dodIDQuery, lastNameQuery, dutyStationQuery, moveStatusQuery, gblocQuery, sortOrderQuery} - query := f.db.Q().Eager( + query := f.db.Q().EagerPreload( "Orders.ServiceMember", "Orders.NewDutyStation.Address", - "Orders.OriginDutyStation", + "Orders.OriginDutyStation.Address", + "Orders.OriginDutyStation.TransportationOffice", "Orders.Entitlement", "MTOShipments", "MTOServiceItems", @@ -113,15 +114,6 @@ func (f moveOrderFetcher) ListMoveOrders(officeUserID uuid.UUID, params *service // Get the count count := query.Paginator.TotalEntriesSize - for i := range moves { - // Due to a bug in pop (https://github.com/gobuffalo/pop/issues/578), we - // cannot eager load the address as "OriginDutyStation.Address" because - // OriginDutyStation is a pointer. - if moves[i].Orders.OriginDutyStation != nil { - f.db.Load(moves[i].Orders.OriginDutyStation, "Address", "TransportationOffice") - } - } - return moves, count, nil } From c1bd499da8378e3df690a3aaf98b41dcb0467bf8 Mon Sep 17 00:00:00 2001 From: Reggie Riser Date: Wed, 10 Feb 2021 16:03:38 +0000 Subject: [PATCH 3/5] Doing separate Pop Load call to workaround EagerPreload bug --- pkg/services/move_order/move_order_fetcher.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/pkg/services/move_order/move_order_fetcher.go b/pkg/services/move_order/move_order_fetcher.go index 3f8fc0c1660..11b9a7b7a3b 100644 --- a/pkg/services/move_order/move_order_fetcher.go +++ b/pkg/services/move_order/move_order_fetcher.go @@ -61,7 +61,8 @@ func (f moveOrderFetcher) ListMoveOrders(officeUserID uuid.UUID, params *service "Orders.ServiceMember", "Orders.NewDutyStation.Address", "Orders.OriginDutyStation.Address", - "Orders.OriginDutyStation.TransportationOffice", + // See note further below about having to do this in a separate Load call due to a Pop issue. + // "Orders.OriginDutyStation.TransportationOffice", "Orders.Entitlement", "MTOShipments", "MTOServiceItems", @@ -114,6 +115,22 @@ func (f moveOrderFetcher) ListMoveOrders(officeUserID uuid.UUID, params *service // Get the count count := query.Paginator.TotalEntriesSize + for i := range moves { + // There appears to be a bug in Pop for EagerPreload when you have two or more eager paths with 3+ levels + // where the first 2 levels match. For example: + // "Orders.OriginDutyStation.Address" and "Orders.OriginDutyStation.TransportationOffice" + // In those cases, only the last relationship is loaded in the results. So, we can only do one of the paths + // in the EagerPreload above and request the second one explicitly with a separate Load call. + // + // Note that we also had a problem before with Eager as well. Here's what we found with it: + // Due to a bug in pop (https://github.com/gobuffalo/pop/issues/578), we + // cannot eager load the address as "OriginDutyStation.Address" because + // OriginDutyStation is a pointer. + if moves[i].Orders.OriginDutyStation != nil { + f.db.Load(moves[i].Orders.OriginDutyStation, "TransportationOffice") + } + } + return moves, count, nil } From b6c4cc4804fe016d9aace75dc19031fe8dddf5a7 Mon Sep 17 00:00:00 2001 From: Reggie Riser Date: Wed, 10 Feb 2021 16:40:32 +0000 Subject: [PATCH 4/5] Added error check; removed duplicate error processing --- pkg/services/move_order/move_order_fetcher.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/pkg/services/move_order/move_order_fetcher.go b/pkg/services/move_order/move_order_fetcher.go index 11b9a7b7a3b..132aea52275 100644 --- a/pkg/services/move_order/move_order_fetcher.go +++ b/pkg/services/move_order/move_order_fetcher.go @@ -81,14 +81,6 @@ func (f moveOrderFetcher) ListMoveOrders(officeUserID uuid.UUID, params *service } } - if err != nil { - switch err { - case sql.ErrNoRows: - return []models.Move{}, 0, services.NotFoundError{} - default: - return []models.Move{}, 0, err - } - } // Pass zeros into paginate in this case. Which will give us 1 page and 20 per page respectively if params.Page == nil { params.Page = swag.Int64(0) @@ -127,7 +119,10 @@ func (f moveOrderFetcher) ListMoveOrders(officeUserID uuid.UUID, params *service // cannot eager load the address as "OriginDutyStation.Address" because // OriginDutyStation is a pointer. if moves[i].Orders.OriginDutyStation != nil { - f.db.Load(moves[i].Orders.OriginDutyStation, "TransportationOffice") + loadErr := f.db.Load(moves[i].Orders.OriginDutyStation, "TransportationOffice") + if loadErr != nil { + return []models.Move{}, 0, err + } } } From ace8ff63aa6dfe04fd421cf160bdddbb23682759 Mon Sep 17 00:00:00 2001 From: Reggie Riser Date: Wed, 10 Feb 2021 16:41:30 +0000 Subject: [PATCH 5/5] Moved payment request queue to EagerPreload; still working around Pop bug --- .../payment_request_list_fetcher.go | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/pkg/services/payment_request/payment_request_list_fetcher.go b/pkg/services/payment_request/payment_request_list_fetcher.go index 739688df3de..989dee58446 100644 --- a/pkg/services/payment_request/payment_request_list_fetcher.go +++ b/pkg/services/payment_request/payment_request_list_fetcher.go @@ -48,9 +48,10 @@ func (f *paymentRequestListFetcher) FetchPaymentRequestList(officeUserID uuid.UU } paymentRequests := models.PaymentRequests{} - query := f.db.Q().Eager( - "MoveTaskOrder.Orders.OriginDutyStation", - "MoveTaskOrder.Orders.ServiceMember", + query := f.db.Q().EagerPreload( + "MoveTaskOrder.Orders.OriginDutyStation.TransportationOffice", + // See note further below about having to do this in a separate Load call due to a Pop issue. + // "MoveTaskOrder.Orders.ServiceMember", ). InnerJoin("moves", "payment_requests.move_id = moves.id"). InnerJoin("orders", "orders.id = moves.orders_id"). @@ -103,11 +104,19 @@ func (f *paymentRequestListFetcher) FetchPaymentRequestList(officeUserID uuid.UU } for i := range paymentRequests { - // Due to a bug in pop (https://github.com/gobuffalo/pop/issues/578), we - // cannot eager load the address as "OriginDutyStation.Address" because - // OriginDutyStation is a pointer. - if originDutyStation := paymentRequests[i].MoveTaskOrder.Orders.OriginDutyStation; originDutyStation != nil { - f.db.Load(originDutyStation, "TransportationOffice") + // There appears to be a bug in Pop for EagerPreload when you have two or more eager paths with 3+ levels + // where the first 2 levels match. For example: + // "MoveTaskOrder.Orders.OriginDutyStation.TransportationOffice" and "MoveTaskOrder.Orders.ServiceMember" + // In those cases, only the last relationship is loaded in the results. So, we can only do one of the paths + // in the EagerPreload above and request the second one explicitly with a separate Load call. + // + // Note that we also had a problem before with Eager as well. Here's what we found with it: + // Due to a bug in pop (https://github.com/gobuffalo/pop/issues/578), we + // cannot eager load the address as "OriginDutyStation.Address" because + // OriginDutyStation is a pointer. + loadErr := f.db.Load(&paymentRequests[i].MoveTaskOrder.Orders, "ServiceMember") + if loadErr != nil { + return nil, 0, err } }