Update README and code comments #391

Charles-Gagnon · 2022-10-11T22:45:55Z

Closes #283
Closes #303

chlafreniere · 2022-10-11T22:49:21Z

src/TriggerBinding/SqlTableChangeMonitor.cs

+                    // Either there's a bug or we're in a bad state so not much we can do here. We'll try clearing
+                    //  our state and retry getting the changes from the top again in case something broke while
+                    // fetching the changes.
+                    // It doesn't make sense to retry processing the changes immediately since this isn't a connection based issue.


super nit: connection-based

chlafreniere · 2022-10-11T22:50:11Z

README.md

+
+e.g. `Leases_7d12c06c6ddff24c_1845581613`
+
+This table is used to ensure that all changes are processed and that no change is processed more than once. This table consists of two groups of columns :


nit: extra space before column

chlafreniere · 2022-10-11T22:50:25Z

README.md

+
+In addition a schema named `az_func` will be created that the tables will belong to.
+
+The login the trigger is configured to use must be given permissions to create these tables and schema. If not then an error will be thrown and the trigger will fail to be ran.


will fail to run

chlafreniere · 2022-10-11T22:50:47Z

README.md

+
+The login the trigger is configured to use must be given permissions to create these tables and schema. If not then an error will be thrown and the trigger will fail to be ran.
+
+If the tables are deleted or modified then unexpected behavior may occur. To reset the state of the triggers first stop all currently running functions with trigger bindings and then either truncate or delete the tables - the next time a function with a trigger binding is started it will recreate the tables as necessary.


chlafreniere · 2022-10-11T22:50:58Z

README.md

+
+The trigger functionality creates a number of tables to use for tracking the current state of the trigger. This allows state to be persisted across sessions and for multiple instances of a trigger binding to execute in parallel (for scaling purposes).
+
+In addition a schema named `az_func` will be created that the tables will belong to.


chlafreniere · 2022-10-11T22:53:11Z

README.md

+
+##### azfunc.Leases_*
+
+A `Leases_*` table is created for every unique instance of a function and table. The full name will be in the format `Leases_<FunctionId>_<TableId>` where `<FunctionId>` is a hash of the Host ID assigned by the function runtime and `<TableId>` is the SQL Object ID of the table being tracked.


Do leases tables ever get cleaned up?

Yes - after every function invocation we clean up old rows from previous sync versions.

azure-functions-sql-extension/src/TriggerBinding/SqlTableChangeMonitor.cs

Line 852 in a897d7c

DELETE FROM {this._leasesTableName} WITH (TABLOCKX) WHERE {SqlTriggerConstants.LeasesTableChangeVersionColumnName} <= {newLastSyncVersion};

README.md

JatinSanghvi · 2022-10-12T08:44:39Z

README.md

+
+The login the trigger is configured to use must be given permissions to create these tables and schema. If not then an error will be thrown and the trigger will fail to run.
+
+If the tables are deleted or modified, then unexpected behavior may occur. To reset the state of the triggers first stop all currently running functions with trigger bindings and then either truncate or delete the tables - the next time a function with a trigger binding is started it will recreate the tables as necessary.


Put commas in:

If not, then an error will be thrown or use 'Otherwise'.

To reset the state of the triggers, first stop all currently running functions

the next time a function with a trigger binding is started, it will recreate the tables as necessary.

Done (sorta - I basically copied everything into word and just took the suggestions there to add commas where it suggested)

JatinSanghvi · 2022-10-12T08:48:47Z

README.md

+##### azfunc.Leases_*
+
+A `Leases_*` table is created for every unique instance of a function and table. The full name will be in the format `Leases_<FunctionId>_<TableId>` where `<FunctionId>` is a hash of the Host ID assigned by the function runtime and `<TableId>` is the SQL Object ID of the table being tracked.
+


I am not sure if the implementation details will benefit the users reading the Readme file. We can instead mention that the leases table's names are based on the user function and the table's object ID.

JatinSanghvi · 2022-10-12T08:54:53Z

README.md

+
+A `Leases_*` table is created for every unique instance of a function and table. The full name will be in the format `Leases_<FunctionId>_<TableId>` where `<FunctionId>` is a hash of the Host ID assigned by the function runtime and `<TableId>` is the SQL Object ID of the table being tracked.
+
+e.g. `Leases_7d12c06c6ddff24c_1845581613`


Can we not have a separate paragraph for the example and just merge it with the paragraph above?

Addressed comments

* Update README and code comments * more * PR comments * Add cleanup docs * Fix spelling * pr comments

Charles-Gagnon added 2 commits October 11, 2022 15:41

Update README and code comments

95e7963

more

38242da

Charles-Gagnon requested review from AmeyaRele and JatinSanghvi October 11, 2022 22:45

Charles-Gagnon requested review from MaddyDev, chlafreniere and lucyzhang929 as code owners October 11, 2022 22:45

chlafreniere reviewed Oct 11, 2022

View reviewed changes

Charles-Gagnon added 3 commits October 11, 2022 16:37

PR comments

a897d7c

Add cleanup docs

ccfa1c5

Fix spelling

e381b01

JatinSanghvi previously requested changes Oct 12, 2022

View reviewed changes

pr comments

6e964dd

chlafreniere approved these changes Oct 12, 2022

View reviewed changes

Charles-Gagnon requested a review from JatinSanghvi October 12, 2022 17:20

Charles-Gagnon merged commit 72751fe into triggerbindings Oct 12, 2022

Charles-Gagnon deleted the chgagnon/triggerDocs branch October 12, 2022 17:21

PBBlox pushed a commit to PBBlox/azure-functions-sql-extension that referenced this pull request Apr 6, 2025

Update README and code comments (Azure#391)

a9012b8

* Update README and code comments * more * PR comments * Add cleanup docs * Fix spelling * pr comments


		e.g. `Leases_7d12c06c6ddff24c_1845581613`

		This table is used to ensure that all changes are processed and that no change is processed more than once. This table consists of two groups of columns :


		In addition a schema named `az_func` will be created that the tables will belong to.

		The login the trigger is configured to use must be given permissions to create these tables and schema. If not then an error will be thrown and the trigger will fail to be ran.


		The login the trigger is configured to use must be given permissions to create these tables and schema. If not then an error will be thrown and the trigger will fail to be ran.

		If the tables are deleted or modified then unexpected behavior may occur. To reset the state of the triggers first stop all currently running functions with trigger bindings and then either truncate or delete the tables - the next time a function with a trigger binding is started it will recreate the tables as necessary.


		The trigger functionality creates a number of tables to use for tracking the current state of the trigger. This allows state to be persisted across sessions and for multiple instances of a trigger binding to execute in parallel (for scaling purposes).

		In addition a schema named `az_func` will be created that the tables will belong to.


		##### azfunc.Leases_*

		A `Leases_*` table is created for every unique instance of a function and table. The full name will be in the format `Leases_<FunctionId>_<TableId>` where `<FunctionId>` is a hash of the Host ID assigned by the function runtime and `<TableId>` is the SQL Object ID of the table being tracked.


		The login the trigger is configured to use must be given permissions to create these tables and schema. If not then an error will be thrown and the trigger will fail to run.

		If the tables are deleted or modified, then unexpected behavior may occur. To reset the state of the triggers first stop all currently running functions with trigger bindings and then either truncate or delete the tables - the next time a function with a trigger binding is started it will recreate the tables as necessary.

Update README and code comments #391

Update README and code comments #391

Uh oh!

Conversation

Charles-Gagnon commented Oct 11, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants