launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03752
[Merge] lp:~abentley/launchpad/handle-concurrent into lp:launchpad
Aaron Bentley has proposed merging lp:~abentley/launchpad/handle-concurrent into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #648075 in Launchpad itself: "Automatic translations export fails intermittently"
https://bugs.launchpad.net/launchpad/+bug/648075
For more details, see:
https://code.launchpad.net/~abentley/launchpad/handle-concurrent/+merge/62511
= Summary =
Fix bug #648075: Automatic translations export fails intermittently
== Proposed fix ==
This bug appeared to be a transient failure, but was actually because the last_mirrored_id of the branches was stale. This branch fixes it by raising a better exception from DirectBranchError and handling the error in the export script.
== Pre-implementation notes ==
Discussed with Deryck
== Implementation details ==
Fixing the branches requires calling Branch.branchChanged, which takes a bunch of parameters. In order to avoid duplicating code, I moved the parameter generation into get_branch_info and get_db_branch_info.
There was a lot of lint, especially wrt inline comments.
I also renamed Branch.branchChanged's stacked_on_location to stacked_on_url, to match its interface.
== Tests ==
bin/test -t test_raises_StaleLastMirrored -t test_exportToStaleBranch
== Demo and Q/A ==
Hmm. Not sure if this is qa-able, because of how it interacts with branches.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/translations/scripts/tests/test_translations_to_branch.py
lib/lp/code/xmlrpc/codehosting.py
lib/lp/code/errors.py
lib/lp/code/interfaces/branch.py
lib/lp/code/model/directbranchcommit.py
lib/lp/codehosting/vfs/branchfs.py
lib/lp/codehosting/bzrutils.py
lib/lp/code/model/branch.py
lib/lp/translations/scripts/translations_to_branch.py
lib/lp/code/tests/test_directbranchcommit.py
--
https://code.launchpad.net/~abentley/launchpad/handle-concurrent/+merge/62511
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/handle-concurrent into lp:launchpad.
=== modified file 'lib/lp/code/errors.py'
--- lib/lp/code/errors.py 2011-02-14 03:03:17 +0000
+++ lib/lp/code/errors.py 2011-05-26 16:16:46 +0000
@@ -32,6 +32,7 @@
'NoSuchBranch',
'PrivateBranchRecipe',
'ReviewNotPending',
+ 'StaleLastMirrored',
'TooManyBuilds',
'TooNewRecipeFormat',
'UnknownBranchTypeError',
@@ -186,7 +187,7 @@
class BranchMergeProposalExists(InvalidBranchMergeProposal):
"""Raised if there is already a matching BranchMergeProposal."""
- webservice_error(400) #Bad request.
+ webservice_error(400) # Bad request.
class InvalidNamespace(Exception):
@@ -216,6 +217,24 @@
_message_prefix = "No such branch"
+class StaleLastMirrored(Exception):
+ """Raised when last_mirrored_id is out of date with on-disk value."""
+
+ def __init__(self, db_branch, info):
+ """Constructor.
+
+ :param db_branch: The database branch.
+ :param info: A dict of information about the branch, as produced by
+ lp.codehosting.bzrutils.get_branch_info
+ """
+ self.db_branch = db_branch
+ self.info = info
+ Exception.__init__(
+ self,
+ 'Database last_mirrored_id %s does not match on-disk value %s' %
+ (db_branch.last_mirrored_id, self.info['last_revision_id']))
+
+
class PrivateBranchRecipe(Exception):
webservice_error(400)
=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py 2011-05-12 11:39:29 +0000
+++ lib/lp/code/interfaces/branch.py 2011-05-26 16:16:46 +0000
@@ -13,6 +13,7 @@
'BzrIdentityMixin',
'DEFAULT_BRANCH_STATUS_IN_LISTING',
'get_blacklisted_hostnames',
+ 'get_db_branch_info',
'IBranch',
'IBranchBatchNavigator',
'IBranchCloud',
@@ -180,7 +181,7 @@
raise LaunchpadValidationError(structured(message))
if IBranch.providedBy(self.context) and self.context.url == str(uri):
- return # url was not changed
+ return # url was not changed
if uri.path == '/':
message = _(
@@ -407,13 +408,13 @@
title=_("The bug-branch link objects that link this branch "
"to bugs."),
readonly=True,
- value_type=Reference(schema=Interface)) # Really IBugBranch
+ value_type=Reference(schema=Interface)) # Really IBugBranch
linked_bugs = exported(
CollectionField(
title=_("The bugs linked to this branch."),
readonly=True,
- value_type=Reference(schema=Interface))) # Really IBug
+ value_type=Reference(schema=Interface))) # Really IBug
def getLinkedBugTasks(user, status_filter):
"""Return a result set for the tasks that are relevant to this branch.
@@ -428,7 +429,7 @@
@call_with(registrant=REQUEST_USER)
@operation_parameters(
- bug=Reference(schema=Interface)) # Really IBug
+ bug=Reference(schema=Interface)) # Really IBug
@export_write_operation()
@operation_for_version('beta')
def linkBug(bug, registrant):
@@ -440,7 +441,7 @@
@call_with(user=REQUEST_USER)
@operation_parameters(
- bug=Reference(schema=Interface)) # Really IBug
+ bug=Reference(schema=Interface)) # Really IBug
@export_write_operation()
@operation_for_version('beta')
def unlinkBug(bug, user):
@@ -455,12 +456,12 @@
CollectionField(
title=_("Specification linked to this branch."),
readonly=True,
- value_type=Reference(Interface)), # Really ISpecificationBranch
+ value_type=Reference(Interface)), # Really ISpecificationBranch
as_of="beta")
@call_with(registrant=REQUEST_USER)
@operation_parameters(
- spec=Reference(schema=Interface)) # Really ISpecification
+ spec=Reference(schema=Interface)) # Really ISpecification
@export_write_operation()
@operation_for_version('beta')
def linkSpecification(spec, registrant):
@@ -472,7 +473,7 @@
@call_with(user=REQUEST_USER)
@operation_parameters(
- spec=Reference(schema=Interface)) # Really ISpecification
+ spec=Reference(schema=Interface)) # Really ISpecification
@export_write_operation()
@operation_for_version('beta')
def unlinkSpecification(spec, user):
@@ -497,7 +498,7 @@
CollectionField(
title=_("BranchSubscriptions associated to this branch."),
readonly=True,
- value_type=Reference(Interface))) # Really IBranchSubscription
+ value_type=Reference(Interface))) # Really IBranchSubscription
subscribers = exported(
CollectionField(
@@ -614,7 +615,8 @@
merged_revnos=List(Int(
title=_('The target-branch revno of the merge.'))))
@call_with(visible_by_user=REQUEST_USER)
- @operation_returns_collection_of(Interface) # Really IBranchMergeProposal.
+ # Really IBranchMergeProposal
+ @operation_returns_collection_of(Interface)
@export_read_operation()
@operation_for_version('beta')
def getMergeProposals(status=None, visible_by_user=None,
@@ -722,7 +724,7 @@
def associatedSuiteSourcePackages():
"""Return the suite source packages that this branch is linked to.
-
+
:return: A list of suite source packages ordered by pocket.
"""
@@ -771,7 +773,7 @@
code_review_level=Choice(
title=_("The level of code review notification emails."),
vocabulary=CodeReviewNotificationLevel))
- @operation_returns_entry(Interface) # Really IBranchSubscription
+ @operation_returns_entry(Interface) # Really IBranchSubscription
@call_with(subscribed_by=REQUEST_USER)
@export_write_operation()
@operation_for_version('beta')
@@ -794,7 +796,7 @@
person=Reference(
title=_("The person to unsubscribe"),
schema=IPerson))
- @operation_returns_entry(Interface) # Really IBranchSubscription
+ @operation_returns_entry(Interface) # Really IBranchSubscription
@export_read_operation()
@operation_for_version('beta')
def getSubscription(person):
@@ -1042,10 +1044,10 @@
@operation_parameters(
project=Reference(
title=_("The project the branch belongs to."),
- schema=Interface, required=False), # Really IProduct
+ schema=Interface, required=False), # Really IProduct
source_package=Reference(
title=_("The source package the branch belongs to."),
- schema=Interface, required=False)) # Really ISourcePackage
+ schema=Interface, required=False)) # Really ISourcePackage
@export_write_operation()
@operation_for_version('beta')
def setTarget(user, project=None, source_package=None):
@@ -1307,10 +1309,10 @@
@collection_default_content()
def getBranches(limit=50, eager_load=True):
"""Return a collection of branches.
-
- :param eager_load: If True (the default because this is used in the
- web service and it needs the related objects to create links) eager
- load related objects (products, code imports etc).
+
+ :param eager_load: If True (the default because this is used in the
+ web service and it needs the related objects to create links)
+ eager load related objects (products, code imports etc).
"""
@@ -1425,3 +1427,22 @@
return False
celebs = getUtility(ILaunchpadCelebrities)
return user.inTeam(celebs.admin) or user.inTeam(celebs.bazaar_experts)
+
+
+def get_db_branch_info(stacked_on_url, last_revision_id, control_string,
+ branch_string, repository_string):
+ """Return a dict of branch info suitable for Branch.branchChanged.
+
+ :param stacked_on_url: The URL the branch is stacked on.
+ :param last_revision_id: The branch tip revision_id.
+ :param control_string: The control format marker as a string.
+ :param branch_string: The branch format marker as a string.
+ :param repository_string: The repository format marker as a string.
+ """
+ info = {}
+ info['stacked_on_url'] = stacked_on_url
+ info['last_revision_id'] = last_revision_id
+ info['control_format'] = ControlFormat.get_enum(control_string)
+ info['branch_format'] = BranchFormat.get_enum(branch_string)
+ info['repository_format'] = RepositoryFormat.get_enum(repository_string)
+ return info
=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py 2011-05-12 11:39:29 +0000
+++ lib/lp/code/model/branch.py 2011-05-26 16:16:46 +0000
@@ -318,8 +318,8 @@
params = BugTaskSearchParams(user=user, linked_branches=self.id,
status=status_filter)
tasks = shortlist(getUtility(IBugTaskSet).search(params), 1000)
- # Post process to discard irrelevant tasks: we only return one task per
- # bug, and cannot easily express this in sql (yet).
+ # Post process to discard irrelevant tasks: we only return one task
+ # per bug, and cannot easily express this in sql (yet).
return filter_bugtasks_by_context(self.target.context, tasks)
def linkBug(self, bug, registrant):
@@ -593,6 +593,7 @@
result = result.order_by(BranchRevision.sequence)
else:
result = result.order_by(Desc(BranchRevision.sequence))
+
def eager_load(rows):
revisions = map(operator.itemgetter(1), rows)
load_related(RevisionAuthor, revisions, ['revision_author_id'])
@@ -742,8 +743,8 @@
"""Helper for associatedSuiteSourcePackages."""
# This is eager loaded by BranchCollection.getBranches.
series_set = getUtility(IFindOfficialBranchLinks)
- # Order by the pocket to get the release one first. If changing this be
- # sure to also change BranchCollection.getBranches.
+ # Order by the pocket to get the release one first. If changing this
+ # be sure to also change BranchCollection.getBranches.
links = series_set.findForBranch(self).order_by(
SeriesSourcePackageBranch.pocket)
return [link.suite_sourcepackage for link in links]
@@ -1012,17 +1013,17 @@
else:
return getUtility(IBranchLookup).getByUniqueName(location)
- def branchChanged(self, stacked_on_location, last_revision_id,
+ def branchChanged(self, stacked_on_url, last_revision_id,
control_format, branch_format, repository_format):
"""See `IBranch`."""
self.mirror_status_message = None
- if stacked_on_location == '' or stacked_on_location is None:
+ if stacked_on_url == '' or stacked_on_url is None:
stacked_on_branch = None
else:
- stacked_on_branch = self._findStackedBranch(stacked_on_location)
+ stacked_on_branch = self._findStackedBranch(stacked_on_url)
if stacked_on_branch is None:
self.mirror_status_message = (
- 'Invalid stacked on location: ' + stacked_on_location)
+ 'Invalid stacked on location: ' + stacked_on_url)
self.stacked_on = stacked_on_branch
if self.branch_type == BranchType.HOSTED:
self.last_mirrored = UTC_NOW
@@ -1215,7 +1216,7 @@
try:
simplejson.loads(config)
self.merge_queue_config = config
- except ValueError: # The json string is invalid
+ except ValueError: # The json string is invalid
raise InvalidMergeQueueConfig
=== modified file 'lib/lp/code/model/directbranchcommit.py'
--- lib/lp/code/model/directbranchcommit.py 2010-10-29 14:02:52 +0000
+++ lib/lp/code/model/directbranchcommit.py 2011-05-26 16:16:46 +0000
@@ -21,7 +21,11 @@
from canonical.config import config
from canonical.launchpad.interfaces.lpstorm import IMasterObject
-from lp.codehosting.bzrutils import get_stacked_on_url
+from lp.code.errors import StaleLastMirrored
+from lp.codehosting.bzrutils import (
+ get_branch_info,
+ get_stacked_on_url,
+ )
from lp.services.mail.sendmail import format_address_for_person
from lp.services.osutils import override_environ
@@ -99,12 +103,16 @@
self.bzrbranch.lock_write()
self.is_locked = True
-
try:
self.revision_tree = self.bzrbranch.basis_tree()
self.transform_preview = TransformPreview(self.revision_tree)
assert self.transform_preview.find_conflicts() == [], (
"TransformPreview is not in a consistent state.")
+ if not no_race_check:
+ last_revision = self.bzrbranch.last_revision()
+ if last_revision != db_branch.last_mirrored_id:
+ raise StaleLastMirrored(
+ db_branch, get_branch_info(self.bzrbranch))
self.is_open = True
except:
=== modified file 'lib/lp/code/tests/test_directbranchcommit.py'
--- lib/lp/code/tests/test_directbranchcommit.py 2010-12-09 07:14:34 +0000
+++ lib/lp/code/tests/test_directbranchcommit.py 2011-05-26 16:16:46 +0000
@@ -5,12 +5,14 @@
__metaclass__ = type
+from testtools.testcase import ExpectedException
from zope.security.proxy import removeSecurityProxy
from canonical.testing.layers import (
DatabaseFunctionalLayer,
ZopelessDatabaseLayer,
)
+from lp.code.errors import StaleLastMirrored
from lp.code.model.directbranchcommit import (
ConcurrentUpdateError,
DirectBranchCommit,
@@ -119,7 +121,7 @@
committer.last_scanned_id = (
committer.bzrbranch.last_revision())
committer.writeFile('file.txt', 'contents')
- revision_id = committer.commit('')
+ committer.commit('')
branch_revision_id = committer.bzrbranch.last_revision()
branch_revision = committer.bzrbranch.repository.get_revision(
branch_revision_id)
@@ -269,7 +271,7 @@
def test_getDir_creates_dir_in_existing_dir(self):
# _getDir creates directories inside ones that already existed
# in a previously committed version of the branch.
- existing_id = self.committer._getDir('po')
+ self.committer._getDir('po')
self._setUpCommitter()
new_dir_id = self.committer._getDir('po/main/files')
self.assertTrue('po/main' in self.committer.path_ids)
@@ -323,3 +325,20 @@
committer = DirectBranchCommit(branch)
self.addCleanup(committer.unlock)
self.assertIn('noreply', committer.getBzrCommitterID())
+
+
+class TestStaleLastMirroredID(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def test_raises_StaleLastMirrored(self):
+ """Raise if the on-disk revision doesn't match last-mirrored."""
+ self.useBzrBranches(direct_database=True)
+ bzr_id = self.factory.getUniqueString()
+ db_branch, tree = self.create_branch_and_tree()
+ tree.commit('unchanged', committer='jrandom@xxxxxxxxxxx')
+ with ExpectedException(StaleLastMirrored, '.*'):
+ committer = DirectBranchCommit(
+ db_branch, committer_id=bzr_id)
+ self.addCleanup(committer.unlock)
+ committer.commit('')
=== modified file 'lib/lp/code/xmlrpc/codehosting.py'
--- lib/lp/code/xmlrpc/codehosting.py 2011-04-18 01:37:03 +0000
+++ lib/lp/code/xmlrpc/codehosting.py 2011-05-26 16:16:46 +0000
@@ -34,11 +34,6 @@
NotFoundError,
)
from lp.app.validators import LaunchpadValidationError
-from lp.code.bzr import (
- BranchFormat,
- ControlFormat,
- RepositoryFormat,
- )
from lp.code.enums import BranchType
from lp.code.errors import (
BranchCreationException,
@@ -47,6 +42,7 @@
NoLinkedBranch,
UnknownBranchTypeError,
)
+from lp.code.interfaces.branch import get_db_branch_info
from lp.code.interfaces import branchpuller
from lp.code.interfaces.branchlookup import (
IBranchLookup,
@@ -194,6 +190,7 @@
target = IBranchTarget(context)
namespace = target.getNamespace(requester)
branch_name = namespace.findUnusedName('trunk')
+
def link_func(new_branch):
link = ICanHasLinkedBranch(context)
link.setBranch(new_branch, requester)
@@ -274,16 +271,13 @@
if branch is None:
return faults.NoBranchWithID(branch_id)
- control_format = ControlFormat.get_enum(control_string)
- branch_format = BranchFormat.get_enum(branch_string)
- repository_format = RepositoryFormat.get_enum(repository_string)
-
if requester == LAUNCHPAD_SERVICES:
branch = removeSecurityProxy(branch)
- branch.branchChanged(
- stacked_on_location, last_revision_id, control_format,
- branch_format, repository_format)
+ info = get_db_branch_info(
+ stacked_on_location, last_revision_id, control_string,
+ branch_string, repository_string)
+ branch.branchChanged(**info)
return True
=== modified file 'lib/lp/codehosting/bzrutils.py'
--- lib/lp/codehosting/bzrutils.py 2010-10-21 20:44:03 +0000
+++ lib/lp/codehosting/bzrutils.py 2011-05-26 16:16:46 +0000
@@ -11,6 +11,7 @@
__all__ = [
'add_exception_logging_hook',
'DenyingServer',
+ 'get_branch_info',
'get_branch_stacked_on_url',
'get_stacked_on_url',
'get_vfs_format_classes',
@@ -369,6 +370,24 @@
return None
+def get_branch_info(branch):
+ """Get information about the branch for branchChanged.
+
+ :return: a dict containing 'stacked_on_url', 'last_revision_id',
+ 'control_string', 'branch_string', 'repository_string'.
+ """
+ info = {}
+ info['stacked_on_url'] = get_stacked_on_url(branch)
+ info['last_revision_id'] = branch.last_revision()
+ # XXX: Aaron Bentley 2008-06-13
+ # Bazaar does not provide a public API for learning about
+ # format markers. Fix this in Bazaar, then here.
+ info['control_string'] = branch.bzrdir._format.get_format_string()
+ info['branch_string'] = branch._format.get_format_string()
+ info['repository_string'] = branch.repository._format.get_format_string()
+ return info
+
+
@contextmanager
def read_locked(branch):
branch.lock_read()
=== modified file 'lib/lp/codehosting/vfs/branchfs.py'
--- lib/lp/codehosting/vfs/branchfs.py 2011-03-29 03:24:05 +0000
+++ lib/lp/codehosting/vfs/branchfs.py 2011-05-26 16:16:46 +0000
@@ -105,7 +105,10 @@
CONTROL_TRANSPORT,
LAUNCHPAD_SERVICES,
)
-from lp.codehosting.bzrutils import get_stacked_on_url
+from lp.codehosting.bzrutils import (
+ get_branch_info,
+ get_stacked_on_url,
+ )
from lp.codehosting.vfs.branchfsclient import (
BranchFileSystemClient,
)
@@ -689,23 +692,16 @@
try:
branch = BzrDir.open_from_transport(transport).open_branch(
ignore_fallbacks=True)
- last_revision = branch.last_revision()
- stacked_on_url = self._normalize_stacked_on_url(branch)
- # XXX: Aaron Bentley 2008-06-13
- # Bazaar does not provide a public API for learning about
- # format markers. Fix this in Bazaar, then here.
- control_string = branch.bzrdir._format.get_format_string()
- branch_string = branch._format.get_format_string()
- repository_string = \
- branch.repository._format.get_format_string()
+ info = get_branch_info(branch)
+ info['stacked_on_url'] = (
+ self._normalize_stacked_on_url(branch))
finally:
if jail_info.transports:
jail_info.transports.remove(transport)
- if stacked_on_url is None:
- stacked_on_url = ''
+ if info['stacked_on_url'] is None:
+ info['stacked_on_url'] = ''
return self._branchfs_client.branchChanged(
- data['id'], stacked_on_url, last_revision,
- control_string, branch_string, repository_string)
+ data['id'], **info)
@no_traceback_failures
def handle_error(failure=None, **kw):
=== modified file 'lib/lp/translations/scripts/tests/test_translations_to_branch.py'
--- lib/lp/translations/scripts/tests/test_translations_to_branch.py 2010-12-21 19:42:00 +0000
+++ lib/lp/translations/scripts/tests/test_translations_to_branch.py 2011-05-26 16:16:46 +0000
@@ -9,6 +9,7 @@
from textwrap import dedent
from bzrlib.errors import NotBranchError
+from testtools.matchers import MatchesRegex
import transaction
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
@@ -142,6 +143,30 @@
self.assertEqual(
None, re.search("INFO\s+Committed [0-9]+ file", stderr))
+ def test_exportToStaleBranch(self):
+ # Attempting to export to a stale branch marks it for scanning.
+ self.useBzrBranches(direct_database=False)
+ exporter = ExportTranslationsToBranch(test_args=[])
+ exporter.logger = BufferLogger()
+ productseries = self.factory.makeProductSeries()
+ db_branch, tree = self.create_branch_and_tree(
+ product=productseries.product)
+ removeSecurityProxy(productseries).translations_branch = db_branch
+ db_branch.last_mirrored_id = 'stale-id'
+ db_branch.last_scanned_id = db_branch.last_mirrored_id
+ self.becomeDbUser('translationstobranch')
+ self.assertFalse(db_branch.pending_writes)
+ self.assertNotEqual(
+ db_branch.last_mirrored_id, tree.branch.last_revision())
+ exporter._exportToBranch(productseries)
+ self.assertEqual(
+ db_branch.last_mirrored_id, tree.branch.last_revision())
+ self.assertTrue(db_branch.pending_writes)
+ matches = MatchesRegex(
+ '(.|\n)*WARNING Skipped .* due to stale DB info and scheduled'
+ ' scan.')
+ self.assertThat(exporter.logger.getLogBuffer(), matches)
+
def test_exportToBranches_handles_nonascii_exceptions(self):
# There's an exception handler in _exportToBranches that must
# cope well with non-ASCII exception strings.
=== modified file 'lib/lp/translations/scripts/translations_to_branch.py'
--- lib/lp/translations/scripts/translations_to_branch.py 2010-12-23 17:30:02 +0000
+++ lib/lp/translations/scripts/translations_to_branch.py 2011-05-26 16:16:46 +0000
@@ -34,6 +34,8 @@
SLAVE_FLAVOR,
)
from lp.app.enums import ServiceUsage
+from lp.code.errors import StaleLastMirrored
+from lp.code.interfaces.branch import get_db_branch_info
from lp.code.interfaces.branchjob import IRosettaUploadJobSource
from lp.code.model.directbranchcommit import (
ConcurrentUpdateError,
@@ -180,7 +182,6 @@
template.date_last_updated > changed_since):
yield pofile
-
def _exportToBranch(self, source):
"""Export translations for source into source.translations_branch.
@@ -189,7 +190,17 @@
self.logger.info("Exporting %s." % source.title)
self._checkForObjections(source)
- committer = self._prepareBranchCommit(source.translations_branch)
+ try:
+ committer = self._prepareBranchCommit(source.translations_branch)
+ except StaleLastMirrored, e:
+ source.translations_branch.branchChanged(
+ **get_db_branch_info(**e.info))
+ self.logger.warning(
+ 'Skipped %s due to stale DB info and scheduled scan.',
+ source.translations_branch.bzr_identity)
+ if self.txn:
+ self.txn.commit()
+ return
self.logger.debug("Created DirectBranchCommit.")
if self.txn:
self.txn.commit()
