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

inconsistent transaction state #184

Closed
rtxu opened this issue Mar 17, 2015 · 21 comments
Closed

inconsistent transaction state #184

rtxu opened this issue Mar 17, 2015 · 21 comments

Comments

@rtxu
Copy link

rtxu commented Mar 17, 2015

First of all,I open the db file in the following way.

db, err := sql.Open("sqlite3", "file:locked.sqlite?cache=shared&mode=rwc")

My program commit a db transaction periodically. At the same time, another program(like /usr/bin/sqlite3) do a big query on the same db. Then error reported is:

2015/03/16 20:05:49 "database is locked": Commit
# Error from the successive transation
2015/03/16 20:05:49 "cannot start a transaction within a transaction": Begin transaction

IMO, a fail-commited transaction means a rollbacked transaction. Am I wrong ?
I do the rollback manually. And error reported became:

2015/03/16 20:06:51 "database is locked": Commit
2015/03/16 20:06:51 "sql: Transaction has already been committed or rolled back": Rollback
2015/03/16 20:06:51 "cannot start a transaction within a transaction": Begin transaction

Now, I‘m confusing. What should I do to begin a new transaction? The old transaction is rolled back, OR NOT?

Do not inform me: Close-Then-ReOpen the db. It's just a workaround.

func Open

func Open(driverName, dataSourceName string) (*DB, error)

Open opens a database specified by its database driver name and a driver-specific data source name, usually consisting of at least a database name and connection information.

Most users will open a database via a driver-specific connection helper function that returns a *DB. No database drivers are included in the Go standard library. See http://golang.org/s/sqldrivers for a list of third-party drivers.

Open may just validate its arguments without creating a connection to the database. To verify that the data source name is valid, call Ping.

The returned DB is safe for concurrent use by multiple goroutines and maintains its own pool of idle connections. Thus, the Open function should be called just once. It is rarely necessary to close a DB.

Finally I need help and also want to help.

@mattn
Copy link
Owner

mattn commented Mar 17, 2015

Could you please show me the small code to reproduce?

@rtxu
Copy link
Author

rtxu commented Mar 17, 2015

Here's my original code snippet:
https://gist.github.com/xrtgavin/5a18126c383cfc29ccb4/5446f1877facc57e889f8bb036794c9baf3356b7#file-gistfile1-go

REMIND:When commit transaction periodically, /usr/bin/sqlite3 will do a big query and the commit will fail.

Here's modified code snippet:
https://gist.github.com/xrtgavin/5a18126c383cfc29ccb4/3786e63e4783cd23126b3b2915563424882d4a9e#file-gistfile1-go

Here's the Close-Then-ReOpen workaround I'm not satisfied:
https://gist.github.com/xrtgavin/5a18126c383cfc29ccb4#file-gistfile1-go

@rtxu
Copy link
Author

rtxu commented Mar 20, 2015

@mattn What can I do for you?

@mattn
Copy link
Owner

mattn commented Mar 20, 2015

I'm thinking either of program should be given a BUSY or LOCKED at commit time.

@rtxu
Copy link
Author

rtxu commented Apr 8, 2015

How the bug progress?

@mattn
Copy link
Owner

mattn commented Apr 8, 2015

Could you please try _busy_timeout?

https://github.com/mattn/go-sqlite3/blob/master/sqlite3.go#L274

Set bigger value like _busy_timeout=XXXXX.

@rtxu
Copy link
Author

rtxu commented Apr 8, 2015

When timeout, could I fail the current transaction and begin another transaction without reopen db?

@mattn
Copy link
Owner

mattn commented Apr 8, 2015

Do you know where the timeout occured? Commit? Exec?

@rtxu
Copy link
Author

rtxu commented Apr 8, 2015

Almost commit, I guess, because it's the most time-consuming operation, isn't it ?

And _busy_timeout's unit ? ms or us ?

@mattn
Copy link
Owner

mattn commented Apr 8, 2015

And _busy_timeout's unit ? ms or us ?

ms
see https://www.sqlite.org/c3ref/busy_timeout.html

Almost commit, I guess, because it's the most time-consuming operation, isn't it ?

I must check which is occured timeout.

@mattn
Copy link
Owner

mattn commented Apr 8, 2015

Could you please minimum test case that can reproduce?

@rtxu
Copy link
Author

rtxu commented Apr 8, 2015

No matter what I set timeout is, I have to reopen db when transaction fail, right ?

@mattn
Copy link
Owner

mattn commented Apr 8, 2015

I want to check whether this behavior should be fixed or not.

@xeodou
Copy link

xeodou commented May 18, 2015

hi @xrtgavin Here is what i solve the database is locked issue

func (s *Meyovoter) InsertFormVoterList(mid int64, ids []int64) error {
    tx, err := s.db.Begin()
    if err != nil {
        return err
    }
    str := fmt.Sprintf("INSERT INTO %s ( `meyo_id`, `voter_id`) VALUES (?, ?);", meyo_voter_table)

    stmt, err := tx.Prepare(str)
    if err != nil {
        return err
    }
    defer stmt.Close()
    for _, id := range ids {
        _, err = stmt.Exec(mid, id)
        if err != nil {
            tx.Rollback() // This line is the key.
            return err
        }
    }
    if err := tx.Commit(); err != nil {
        return tx.Rollback()
    }
    return nil
}

@rtxu
Copy link
Author

rtxu commented Jun 11, 2015

@xeodou Rollback() does not make sense to the problem.

See my problem again.

@xeodou
Copy link

xeodou commented Jun 11, 2015

oh, Sorry. @xrtgavin

@ea-at-diladele-com
Copy link

Hi guys,
Any progress on this?
It seems I have bumped into the same issue. Is db reopening the only workaround?
Thank you!

@rtxu
Copy link
Author

rtxu commented Jan 8, 2016

@ea-at-diladele-com AFAIK,no any process

@sqweek
Copy link
Contributor

sqweek commented Apr 16, 2016

@xrtgavin wrote:

IMO, a fail-commited transaction means a rollbacked transaction. Am I wrong ?

You are wrong from sqlite's perspective. See https://www.sqlite.org/lang_transaction.html:

An attempt to execute COMMIT might also result in an SQLITE_BUSY return code if an another thread or process has a shared lock on the database that prevented the database from being updated. When COMMIT fails in this way, the transaction remains active and the COMMIT can be retried later after the reader has had a chance to clear.

However you are correct from database/sql's perspective - it closes the transaction whether COMMIT succeeds or fails.

This is the source of the inconsistency; as far as the database package is concerned the transaction is done, so any further attempts to commit/rollback are met with the error "sql: Transaction has already been committed or rolled back" without consulting the sqlite3 driver.

There's two ways to proceed:

  1. Change go-sqlite3's Commit implementation to always clean up the transaction so it matches the semantics expected by database/sql
  2. Change nothing and leave the user to explicitly clean up the transaction by calling db.Exec("COMMIT") or db.Exec("ROLLBACK") (note db is what was returned from sql.Open, not the transaction).

Option 2 is more flexible in that it allows the user to decide whether they want to retry the COMMIT at a later time, but it also kind of defeats the purpose of using the database/sql API since they have to know (or check) that the sqlite3 driver is in use...

@rtxu
Copy link
Author

rtxu commented Apr 18, 2016

@sqweek Appreciate your insight about the issue. Thank you.

sqweek added a commit to sqweek/go-sqlite3 that referenced this issue Apr 18, 2016
The semantics of sql.Tx.Commit impose that the transaction is
finished and cleaned up by the time the driver's Commit function
returns. However sqlite3 leaves the transaction open if COMMIT
fails due to an SQLITE_BUSY error, so *we* must clean it up.

Closes mattn#184.
@sqweek
Copy link
Contributor

sqweek commented Apr 18, 2016

I think option 1 is the right solution; it seems like the only way to provide consistent behaviour via the database/sql.Tx API. I've opened a pull request to that effect.

Users who really really want to keep retrying COMMIT upon SQLITE_BUSY can either:

  1. Set the busy timeout to something ridiculously large, or
  2. Manually manage their transactions (db.Exec("BEGIN"), db.Exec("COMMIT"), db.Exec("ROLLBACK"))

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

No branches or pull requests

5 participants