Web Client - Check In List Refreshes After Every Scan and Gets Progressively Slower

[Jennifer Pringle]

Evergreen 3.1

The check in list continues to refresh after every item scanned and takes longer to refresh the more items there are in the list. We have reports from libraries that scanning an item into the check in screen can take anywhere from 2 seconds to 20 seconds.

It's also misleading that while the list is refreshing it displays "No Items to Display".

Libraries have noted that they don't see the same kind of slowness when scanning items into the In-House screen.

We already have the earlier fix for check in slowness - https://bugs.launchpad.net/evergreen/+bug/1743045

[Terran McCanna (tmccanna)]

One of our larger libraries is reporting a lot of problems with this since they are checking in so many items at a time, particularly when scanning their pulled holds.

[Jason Stephenson (jstephenson)]

I have a question, mainly for people who work circulation:

What utility does the list of checked in items provide? What functionality would you lose if that list were removed from the interface?

Do you use it as a reference of what has been checked in or does it provide some useful function that lets you shortcut other work?

I am asking because I think one simple way to test the cause or a fix for this bug would be to remove the list and that code that tracks the items checked in.

[Terran McCanna (tmccanna)]

In my experience, it's primarily used as a reference of what has been checked in - to see where you are in the stack, to make sure that every item has been scanned, as a secondary method of special statuses in case an alert gets overlooked, etc.

It's also used when patrons want receipts of everything they are turning in.

Staff at every library I've ever worked at and every ILS I've ever worked with would be infuriated if it went away.

[Jennifer Pringle (jpringle-u)]

Our libraries also use the check in list as Terran describes.

Our libraries are currently refreshing the check in screen after every 20 check ins as they find it too slow otherwise. We're currently running 3.1.4.

[John Amundson (jamundson)]

As a workaround in the meantime, we had one library mention they limit their rows to 5 on Check In. I tested this and I did indeed see an increase in speed. (Not a speedy as we'd like, but better!)

Limiting to 5 still allows you access to previously checked in items by paging through the list.

[Jennifer Pringle (jpringle-u)]

You can also view your previously checked in items by changing the number of rows that display and then switching it back to 5 rows before checking in again.

[Andrea Neiman (aneiman)]

CW MARS has signed a contract with Equinox to investigate (and hopefully fix) this bug.

[Josh Stompro (u-launchpad-stompro-org)]

I noticed that bug 1697954 comment 11 mentions a fix for extra grid refreshes in the items out screen. Maybe the fix for that is applicable to this bug?
https://bugs.launchpad.net/evergreen/+bug/1697954/comments/11

[Galen Charlton (gmc)]

Josh: thanks for the pointer. However, in the case of the checkin grid, the problem is not that data are being repeatedly re-fetched; instead, as more entries get added to the grid, it's been taking progressively longer for AngularJS to render the grid rows from scratch.

[Galen Charlton (gmc)]

The working repo branch collab/gmcharlt/lp1777207_circ_grid_performance contains a patch that significantly improves performance of the checkin and checkout grids by teaching egGrid how to simply prepend the new row representing the circulation action, rather than doing a full grid refresh.

Feedback welcome. I'm also taking a look at the underlying performance issue, since when you do have to do a full refresh, like you might when changing the number of rows displayed, it's still not really acceptable that it can take a few seconds to render > 20 rows in the checkin grid when no additional data retrieval is needed.

[Jennifer Pringle (jpringle-u)]

Tested Galen's patch on our testing server running 3.1.7 with one of our libraries.

The lag between check ins has improved by 1 or 2 seconds which now makes each check in take about 2 to 3 seconds (rather than 4 to 5 seconds). The lack of flashing (from when it refreshed each time) is great! Based on our brief testing the lag is also significantly decreased between a check in and the hold or transit pop-up appearing.

[Galen Charlton (gmc)]

Thanks for testing, Jennifer!

I've pushed a second patch to the branch that tweaks eg-grid to generate DOM nodes for grid cells only for visible columns, instead of all columns. While this shaves a bit of time when checking in an item. its effect is much more apparent when changing the row limit in a populated grid.

[Jennifer Pringle (jpringle-u)]

Further testing (of the original patch) on our customized 3.1.7 server and on a 3.1.7 server using the Concerto data set without any of our customizations has revealed a couple issues when columns are sorted in Check In.

1. If you click on any of the column labels and then check items in once you reach the total visible rows new check ins no longer appear on your visible list. If you switch to having more rows visible your check ins are in the list. I can consistently replicate this. (On a 3.1.7 server without the patch the most recent check in always appears in the visible rows.)

2. Additional, if you click on any of the column labels and then check items sometimes check appears to stop working completely. When an item is scanned you hear the success audio sign but your item doesn't appear on the check in list (and isn't there if you expand to see more rows). In the console you can see an error message that begins with "Error: [ngRepeat:dupes] Duplicates in a repeater are not allowed. Use 'track by' expression to specify unique keys. Repeater: item in items, Duplicate key:" Despite not showing up in the check in list the item does show as checked in if you look at it in Item Status. When you run into this issue all additional scans fail to appear in the list and the only way to resolve this is to open a fresh Check In (in a different or the same tab).

#2 appears to occur randomly, but I have been able to consistently replicate it by clicking on a column label (like barcode) and then checking in two items that are attached to the same bibliographic record. However, sometimes it happens with items not attached to the same bibliographic record.

If there is no data in a column it doesn't appear to cause the same issue, ie. clicking on Checkin Date if you're checking in available items which have no check in date.

[Galen Charlton (gmc)]

> 1. If you click on any of the column labels and then check items in once
> you reach the total visible rows new check ins no longer appear on your
> visible list. If you switch to having more rows visible your check ins
> are in the list. I can consistently replicate this. (On a 3.1.7 server
> without the patch the most recent check in always appears in the visible
> rows.)

Clicking on a column heading sorts by that column's values, and the new
prepend() method doesn't remove any active sort values. As a consequence,
new checkouts might not be visible due to the current sort order; e.g., if
you've sorted the table by shelving location, and check in an item whose
location is in the ZZ Top Collection, it might be pushed out the of visible
range. I tested in stock 3.1.x, and this behavior appears to be the same
without that patch, by the way. It's a pity that the AngularJS grid doesn't
indicate the current sorting state.

I suppose that prepend() could be adjusted to remove any user-specified
sort order (and we'd probably want to make a grid option for that). Would
that behavior make sense for the checkin grid?

> 2. Additional, if you click on any of the column labels and then check
> items sometimes check appears to stop working completely. When an item
> is scanned you hear the success audio sign but your item doesn't appear
> on the check in list (and isn't there if you expand to see more rows).
> In the console you can see an error message that begins with "Error:
> [ngRepeat:dupes] Duplicates in a repeater are not allowed. Use 'track
> by' expression to specify unique keys.

I'll investigate that.

[Jennifer Pringle (jpringle-u)]

Just to note, I get the same errors in checkout as I see in check in if I click on a column header to sort. The items do check out even though they don't appear on the screen and the number on the Items Out tab increases with each check out.

[Jennifer Pringle (jpringle-u)]

We have now been able to re-create issue #2 another way. If you scan multiple barcodes into check in quite quickly only the first one or two show up and the same error message shows up in the console.

[Mike Rylander (mrylander)]

I wonder if we shouldn't simply remove sorting from checkin and checkout? That may be heretical to say, and please correct me if it's very important, but because those interfaces are essentially a chronological log of actions taken, preserving that order (even at all costs) seems reasonable to me.

[Jeff Davis (jdavis-sitka)]

I think sort is very useful and should be retained, especially on checkin. Lots of libraries prefer to keep a single checkin tab open all day with an ever-growing list of checkins, because it helps them manage the items that came in at that workstation; sorting makes a lengthy list manageable. Ascending sort on checkin date is the easiest way to review checkins in the order they occurred. You might want to sort by shelving location for reshelving, by user to help with patron inquiries, or by routing destination to manage transits.

For checkout, I've heard that sort is used to control the order of items on checkout receipts.

[Galen Charlton (gmc)]

I've pushed two more patches to the branch. The first resolves the bug with fast scans or sorting causing the duplicate entry error, while the second teaches prepend() how to reset sort columns (which is needed to avoid having to do full collect()'s all the time if a sort order is in effect). From the description of the second patch:

    On the checkin and checkout grids, due to a quirk of how
    sorting in arrayNotifier-based data sources work, the grids
    will behave like this:

    - User processes a bunch of items, with each new one
      showing up at the top of the list
    - User sorts the grid to look at something
    - User processes another item. A collect() happens.
      The new item will show up at the top of the list, with the remaining
      items following the previous sort order.
    - User processes more items; the prepending will coninue
      and remain fast.

That second patch is option if we want the last sort order to be strictly enforced, but I think it arrive at a good compromise between having prepends in order to make the checkin and checkout grids faster while continuing to allow operators to use sorting.

[Jennifer Pringle (jpringle-u)]

I've now tested all four patches together on our testing server using a barcode scanner for entering items in check in and check out. The issues I encounter after the first two patches are resolved and I'm not encountering any further issues.

[Dan Wells (dbw2)]

I've looked over the code, and it looks good. Just a few questions/comments.

First, I think the sort reset added in the last commit is a good idea and should simply be the default, non-optional behavior, similar to resetting the offset to 0. Like the offset reset, the whole concept of "prepending" depends upon the sort not affecting new results. Otherwise, we are doing some sort of "insert" behavior which I think is always going to be a bad UX for a grid, i.e. a visual hunt for the new row on every action.

Second, I don't quite understand yet why we must do the collect() in the post-sort/sort-reset prepend case. I imagine it is needed and plan to poke some more to understand why, but if someone more familiar can guess what detail I might be missing and help me understand, I would be most appreciative.

Our staff will be very happy with this fix. Thanks, Galen!

[John Amundson (jamundson)]

I had a chance to test this for CW MARS. We are seeing highly improved check in times, especially when more items are on the screen.

I ran some timings pre-patch:
I checked in 28 items on a Check In screen limited to 100 rows. I suppressed holds and transits, so these are "best case" times.
Items 1 through 10 took between 2 and 3 seconds to check in.
Items 10 through 20 took between 3 and 6 seconds to check in
Items 20 through 25 took between 6 and 7 seconds to check in
Items above 25 took between 8 and 9 seconds to check in. When I reduced the row limit to 5, this dropped down to 3 seconds.

Post-patch:
Every check in, regardless of the number of items on the screen, took between 0.5 and 1.5 seconds. When I sorted by header and checked in another item, the first check in took 2.5 seconds. Subsequent check ins went back to 1 second.

When I removed the suppression modifier, I recorded the amount of time it took to scan in an item, acknowledge both an alert message and transit pop-up: 7 seconds. All these steps together were still faster than a single check in could be previously.

I like the current behavior where newly scanned items appear at the top of the list even when sorted. Clicking on a header will sort them in whenever desired.

Although I did not record actual times, Check Out appeared to be noticeably faster, as well.

I did not run into any issues in my testing. I confirmed holds were still captured on Check In and that items in odd states, (checked out, in transit, etc), were still handled correctly on Check Out.

I'm not adding my signoff at the moment because I'm not sure if Galen has decided it is ready. Dan Wells also has some unanswered questions.

Perhaps, too, others might want to test.

[Galen Charlton (gmc)]

I've pushed a patch to the collab branch that addresses Dan's questions. In short:

- I'm find with making resetSort be unconditional and have implemented that
- That extra collect() was indeed unnecessary, so I've removed it. However, that works only when arrayNotifier is the data source, so I've edit the comments to state that.

[Galen Charlton (gmc)]

Also, the collab branch is now meant for folks who have been testing over the past few days; I'll shortly prepare a new branch with a squash that should be the one used for the formal signoff.

[Galen Charlton (gmc)]

I've pushed a branch meant for consideration as a pullrequest: user/gmcharlt/lp1777207_grid_prepend_and_circ_grid_performance / https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/gmcharlt/lp1777207_grid_prepend_and_circ_grid_performance

[Dan Wells (dbw2)]

Looks good, works well for me. Thanks for making the extra changes, Galen! Pushed to master through rel_3_1.