search

lightblue-platform / lightblue-migrator

GNU General Public License v3.0
3 stars 13 forks source link

Not using JSON compare for large result sets b/c it consumes a lot of memory #407

Closed paterczm closed 7 years ago

paterczm commented 7 years ago

Service facade uses 2 libraries to diff api responses: JSONCompare and Jiff. Jiff is faster and consumes less memory, but provides less details (stops on first diff encountered and does not describe array element differences). This change will use JSONCompare to report for all responses under 65kB and Jiff for bigger ones (configurable).

bserdar commented 7 years ago

Instead of comparing lenghts, why not use jiff to compare, and then if docs are different, use jsoncompare to get the differences? I think this is the way it is done in migrators.

paterczm commented 7 years ago

Instead of comparing lenghts, why not use jiff to compare, and then if docs are different, use jsoncompare to get the differences? I think this is the way it is done in migrators.

Already doing that, but still running out of memory because of huge inconsistencies in product result sets.

paterczm commented 7 years ago

The logic is following:

  1. Use jiff to compare responses. If equal, return.
  2. If different, check size of responses.
  3. If size < 64kB, use JSONCompare to produce inconsistency (diff) for analysis.
  4. If size >= 64kB, use Jiff to produce inconsistency (diff) for analysis.
bserdar commented 7 years ago

Yes, saw that

On Tue, Mar 21, 2017 at 7:40 AM, Marek notifications@github.com wrote:

The logic is following:

  1. Use jiff to compare responses. If equal, return.
  2. If different, check size of responses.
  3. If size < 64kB, use JSONCompare to produce inconsistency (diff) for analysis.
  4. If size >= 64kB, use Jiff to produce inconsistency (diff) for analysis.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/lightblue-platform/lightblue-migrator/pull/407#issuecomment-288080976, or mute the thread https://github.com/notifications/unsubscribe-auth/ADgDDd0pb7FfuUavZD-ziDHY4k5sn0lSks5rn9NfgaJpZM4MjzmY .

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.3%) to 68.142% when pulling 9ef1f411a8ead9b01a4b4a5a4600769382651b75 on inexpensiveDiff into ec22ec50ed5c1d6e12be882992d35c5b9319795f on master.