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

WIP: End transaction request/response #8

Merged
merged 3 commits into from
Mar 26, 2021

Conversation

peterverraedt
Copy link
Contributor

@peterverraedt peterverraedt commented Mar 19, 2021

This has bitten me and and a couple of colleagues for a day or so.

  1. Make 2 irods connections (e.g. two times session.AcquireConnection)
  2. Make a change using the first connection (e.g. add metadata of an object)
  3. List the metadata using the second connection.
  4. Observe that you still see the situation before change in the first connection. However, when checking metadata with the first connection, or a new fresh connection, you see the updated situation.

One does not see this as long as only one connection from the pool is used.

We learned that

  • irods starts a database transaction on every connection, queries will return the status at the beginning of that transaction
  • most of irods update operations (adding a file, modifing metadata...) will do a database query in the back and commit the transaction
  • some irods operations (bulk metadata changes) require an explicit commit by an endTransactionInp_PI api call

In this PR, I added the end transaction messages, and a Commit/Rollback function. Calling the Commit() function out of the blue is like doing nothing and still force a new database transaction to start and see the most recent changes in the database. This works however only for connections as rodsadmin.

For regular users, a dummy operation is enough. I added PoorMansCommit, but this should be cleaned up.

@peterverraedt
Copy link
Contributor Author

(By the way, we run against mysql, not sure whether postgres would do the same)

@peterverraedt peterverraedt changed the title End transaction request/response WIP: End transaction request/response Mar 22, 2021
@peterverraedt peterverraedt force-pushed the end-transaction branch 2 times, most recently from a9f5ec5 to ca60ded Compare March 22, 2021 16:22
Since the endTransaction API has restricted access, we add the
ModColRequest message and use it for a poor mans version of rollback.
It is crafted such that the server will do nothing except a rollback of
the empty transaction.

Signed-off-by: Peter Verraedt <[email protected]>
We make sure connections retrieved from session.AcquireConnection have
the most recent view on the database.

Signed-off-by: Peter Verraedt <[email protected]>
@iychoi
Copy link
Member

iychoi commented Mar 22, 2021

Thank you for the commit. I'll test it with postgres db. It may take some time to review.

@peterverraedt
Copy link
Contributor Author

peterverraedt commented Mar 25, 2021

FYI my test script. It prints pointers, it is the count of files that matters.

import (
	"fmt"
	"math/rand"
	"time"

	"github.com/cyverse/go-irodsclient/irods/common"
	"github.com/cyverse/go-irodsclient/irods/connection"
	"github.com/cyverse/go-irodsclient/irods/fs"
	"github.com/cyverse/go-irodsclient/irods/types"
)

func test() error {
	conn1, _ := connection.NewIRODSConnection(...)
	conn2, _ := connection.NewIRODSConnection(...)

	printCol(conn1)
	printCol(conn2)

	rand.Seed(time.Now().Unix())

	filename := fmt.Sprintf("/irods_zone/home/rods/test.%d", rand.Uint32())

	fmt.Sprintf("%s\n", filename)

	putInFile(conn1, filename)

	printCol(conn1) // new file is seen
	printCol(conn2) // new file is not seen

	filename2 := fmt.Sprintf("/irods_zone/home/rods/test.%d", rand.Uint32())

	fmt.Sprintf("%s\n", filename2)

	//addMeta(conn2, "/icts_icts/home/rods")

	putInFile(conn2, filename2)

	printCol(conn1) // first new file is seen, second file not
	printCol(conn2) // both new files are seen

	fmt.Print(conn1.PoorMansRollback())

	printCol(conn1) // both new files are seen
	printCol(conn2) // both new files are seen

	conn3, _ := z.AcquireConnection()

	printCol(conn3) // both new files are seen

	return nil
}

func printCol(conn *connection.IRODSConnection) {
	coll, _ := fs.GetCollection(conn, "/irods_zone/home/rods")
	obj, _ := fs.ListDataObjectsMasterReplica(conn, coll)
	fmt.Printf("%v (%d)\n", obj, len(obj))
}

func putInFile(conn *connection.IRODSConnection, filename string) error {
	handle, err := fs.OpenDataObjectWithOperation(conn, filename, "default", "w", common.OPER_TYPE_PUT_DATA_OBJ)
	if err != nil {
		return err
	}

	err = fs.WriteDataObject(conn, handle, []byte("hello\n"))
	if err != nil {
		return err
	}

	return fs.CloseDataObject(conn, handle)
}

func addMeta(conn *connection.IRODSConnection, dirname string) error {
	return fs.AddCollectionMeta(conn, dirname, &types.IRODSMeta{
		Name:  "a",
		Value: fmt.Sprintf("%d", rand.Int()),
	})
}

@iychoi iychoi merged commit 3270222 into cyverse:main Mar 26, 2021
@iychoi
Copy link
Member

iychoi commented Mar 26, 2021

It seems that the issue doesn't occur with postgres db. But doing the rollback didn't harm to the postgres db.
Requesting a dummy operation for every operation may degrade performance for postgres db potentially, so I made a parameter to turn this off.

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

Successfully merging this pull request may close these issues.

2 participants