Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Indexes consistently fail to be created correctly #43

Closed
academy-services opened this issue May 16, 2018 · 6 comments
Closed

Indexes consistently fail to be created correctly #43

academy-services opened this issue May 16, 2018 · 6 comments
Labels

Comments

@academy-services
Copy link

NanoSQL is looking great, congrats. I've plugged it into my app to replace Dexie because it was significantly quicker in testing.

However, I'm upserting some records into IndexedDB through NanoSQL and I'm finding that some rows are not being indexed as expected.

In my test, I have a collection of 4 records that I'm upserting and every time I perform the operation, the 3rd record, no matter what it is, is not indexed.

Here's the model:

  {
    "key": "id",
    "type": "uuid",
    "props": [
      "pk"
    ]
  },
  {
    "key": "client_id",
    "type": "uuid",
    "props": [
      "idx"
    ]
  },
  {
    "key": "client_location_id",
    "type": "uuid",
    "props": [
      "idx"
    ]
  },
  {
    "key": "personnel_id",
    "type": "string",
    "props": [
      "idx"
    ]
  },
  {
    "key": "created_date",
    "type": "string",
    "props": []
  },
  {
    "key": "modified_date",
    "type": "string",
    "props": []
  }
]

This is the data:

  {
    'id': '1',
    'personnel_id': '2596'
  },
  {
    'id': '2',
    'personnel_id': '2596'
  },
  {
    'id': '3',
    'personnel_id': '2596'
  },
  {
    'id': '4',
    'personnel_id': '2596'
  }                                        
];

And this is the code containing the upsert operation:

data.forEach(row => {
  nSQL('personnelLocation').query('upsert', row).exec();             
});

Here's the resulting index in _personnelLocation_idx_personnel_id:

rows: Array(3)
  0: "1"
  1: "2"
  2: "4"

Am I losing my marbles or is there some bizarre issue at play here?

@only-cliches
Copy link
Owner

You're probably losing your marbles, but I doubt that's related to this issue. 😆

So I took everything in your examples above and wasn't able to recreate the issue in a codepen using the latest nanoSQL:

https://codepen.io/clicksimply/pen/jxeyMN

Even enabling IndexedDB didn't cause the issue to come up. Could you help me recreate the issue? Once we see it happening in a codepen fixing it should be easy. 👍

@academy-services
Copy link
Author

Yeah, who am I kidding? Lost my marbles years back.

This pen seems to replicate the issue reliably in Chrome Version 66.0.3359.181:
https://codepen.io/anon/pen/yjREPZ?editors=0010

I changed the upsert to be within a forEach loop as I had it and modified the mode to IDB and the issue is occurring for me consistently. Also changed the database id so that it's not referencing a previously indexed table on an existing db.

I'd work around the issue by dropping the data array into the upsert, but this is part of an orm where each record can save itself to the db, so that won't work so well.

Not sure if this is worth mentioning, but this issue does not occur in Firefox DE in a standard tab. However, in a private Firefox window the codepen throws an error. This same error also appears in Firefox (no private window this time) when testing my app on localhost:

error { target: IDBOpenDBRequest, isTrusted: true, eventPhase: 0, bubbles: true, cancelable: true, defaultPrevented: false, composed: false, timeStamp: 521, cancelBubble: false, originalTarget: IDBOpenDBRequest, … }
console_runner-ce3034e6bde3912cc25f83cccb7caa2b0f976196f2f2d52303a462c826d54a73.js:1:2194
u/</window.console[t]<
https://static.codepen.io/assets/editor/live/console_runner-ce3034e6bde3912cc25f83cccb7caa2b0f976196f2f2d52303a462c826d54a73.js:1:2194
a</e.prototype.onError
https://cdn.jsdelivr.net/npm/nano-sql@1.5.6/dist/nano-sql.min.js:1:16568
Error: nSQL: IndexedDB Error! nano-sql.min.js:1:16585
a</e.prototype.onError
https://cdn.jsdelivr.net/npm/nano-sql@1.5.6/dist/nano-sql.min.js:1:16585

@only-cliches
Copy link
Owner

The secondary index issue is happening because you're running the upserts in parallel. After each upsert the database goes through and makes the needed changes to the indexes before completing the query, when you run all the queries at once you run the risk of the secondary index reads/writes getting out of sync.

Maybe I should setup some sort of queue on the secondary index reads/writes so that if two writes happen at exactly the same time these kinds of issues don't show up.

So temporarily the issue can be resolved by just making sure your writes are chained one after another, instead of done all at once. When you pass an array into the upsert command nanoSQL does this for you.

The IndexedDB issue seems like it's having trouble/unable to open the IndexedDB database. Wha'ts the rest of the debug object at the first line? That might help us figure out what's going on.

@academy-services
Copy link
Author

A queue is probably a good idea. This scenario is likely to pop up again in the future.

Given that NanoSQL was acting as an adapter for my orm where the upserts are called from within the save method of a record, chaining isn't possible without an architecture overhaul and I'm operating on a tight timeframe.

I've switched to RxDB as my adapter and it's working fine.

Regarding that Firefox error - apparently Firefox blocks IndexedDB from within a private window.

Thanks for your assistance, has been much appreciated.

@only-cliches
Copy link
Owner

Awesome, glad you got your project working!

I just pushed v1.5.8 with a queue implemented for the secondary index writes and other updates that usually happen as part of the upsert cycle so this doesn't come up again.

Since you've moved to another project I'll mark this resolved, thanks for the heads up!

@academy-services
Copy link
Author

Just want to say... I'm back using NanoSQL now that this bug is fixed.

It really is a fantastic library, absolutely feature packed and very fast. Beats the competition hands down IMO.

Nice work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants