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

SQLite memory storage broken since 0.12 #1701

Closed
woutertenbosch opened this Issue Sep 27, 2016 · 7 comments

Comments

Projects
None yet
4 participants
@woutertenbosch

woutertenbosch commented Sep 27, 2016

Using SQLite as database and using an in-memory storage, changes are not persistent during the lifetime of the knex object. I use the following code to connect my database:

var knex = require('knex')({
        client: 'sqlite3',
        connection: {filename: ':memory:'}
      });

The knex object can now be used to create tables and store information. The same object can be used to retrieve the stored information. However, if the data is queried at a later time (a few minutes later) using the same knex object, the database is empty again. I guess the problem is the new pooling management in 0.12, causing the connection to be closed after a while, and when the connection is restarted a new in-memory database is constructed. This breaks existing applications using an temporary in-memory store.

If you like, I can write a more extensive script showing what's happening.

@tgriesser

This comment has been minimized.

Owner

tgriesser commented Sep 27, 2016

Oh my bad... I change the pooling from min: 1 to min: 0 for some reason, didn't consider the effect it'd have on :memory: db's. Will fix shortly

@woutertenbosch

This comment has been minimized.

woutertenbosch commented Sep 27, 2016

Oh wow, quick response 👍, was still reading recent commits.

@tgriesser

This comment has been minimized.

Owner

tgriesser commented Sep 27, 2016

Yeah in the meantime if you add/change min: 1, max: 1 in pool config, that should fix things.

@tgriesser tgriesser closed this in b4e6b37 Sep 27, 2016

tgriesser added a commit that referenced this issue Sep 27, 2016

Merge branch '0.12.x'
* 0.12.x:
  release 0.12.2
  Update changelog for 0.12.2 features
  Don't force master as release branch
  Remove unused pool2 dependency
  Fix #1701
  Fix #1675
  Fix for #1691
@tgriesser

This comment has been minimized.

Owner

tgriesser commented Sep 27, 2016

Fixed in 0.12.2... Let me know if you have any issues

@woutertenbosch

This comment has been minimized.

woutertenbosch commented Sep 27, 2016

It did not fix the issue (completely) :(

I did some more investigation into knex and generic-pool and have solved the problem by adding pool: {refreshIdle: false} (in addition to {min:1, max:1} as now present in 0.12.2) to the knex object.

The problem is that the minimum pool size is not enforced by generic-pool if refreshIdle: true. Enabling debug logging on generic-pool showed me that the idle connection is still evicted, after idleTimeoutMillis which is set to 30s by default. Because refreshIdle is true by default, the connection is recreated, causing the memory storage to be lost.

Of course, it makes sense to drop idle connections if the database is on disk to limit the number of open files, so I'm not sure whether the default value of refreshIdle should be adjusted. Maybe this is an issue with generic-pool itself, but I'm not familiar enough with pooling to know for sure. According to the documentation:

refreshIdle: boolean that specifies whether idle resources at or below the min threshold should be destroyed/re-created

Thanks for your help so far!

tgriesser added a commit that referenced this issue Oct 9, 2016

Merge branch 'master' into refactor
* master:
  release 0.12.3
  Update changelog
  Fix #1710
  Fix #1694
  release 0.12.2
  Update changelog for 0.12.2 features
  Don't force master as release branch
  Remove unused pool2 dependency
  Fix #1701
  Fix #1675
  Fix for #1691

RubenSlabbert added a commit to RubenSlabbert/knex that referenced this issue Nov 9, 2016

@zalmoxisus

This comment has been minimized.

zalmoxisus commented Jan 2, 2017

The issue still persists. "pool": { "refreshIdle": false, "min": 1, "max": 1 } doesn't help either.

As noted in interledger-deprecated/five-bells-ledger#371 (comment), after few minutes the database disappears.

Reverting to knex@0.11 helps.

zalmoxisus added a commit to zalmoxisus/remotedev-server that referenced this issue Jan 2, 2017

@devinivy

This comment has been minimized.

Contributor

devinivy commented May 9, 2017

The issue is actually that generic-pool v2 overrides min to be no greater than one less than max. So if you pass configuration { min: 1, max: 1 }, it interprets it as { min: 0, max: 1 }. That's fixed in generic-pool v3, I believe. The fix here is to set the idle timeout for the sole connection to Infinity, so that it never gets reaped as "idle." So, if you're having this issue with knex v0.12 or v0.13 simply add this to your pool configuration: { idleTimeoutMillis: Infinity }.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment