Add the ability to queue definition recomputation #1242
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The version check implementation behaves the same way as the current schema version check.
qtomlinson
changed the title
qtomlinson
changed the title
qtomlinson
changed the title
qtomlinson
force-pushed
the
qt/definition_upgrade
branch
from
b10f052 to
4c00f7b
Compare
Add encoding message in queueing. AzureStorageQueue supports parsing encoded message. Added memoryQueueConfig, to support parsing encoded messages. Also add more logging.
… times within one batch
qtomlinson
force-pushed
the
qt/definition_upgrade
branch
from
fb8b49f to
96938b4
Compare
elrayle
approved these changes
Dec 9, 2024
providers/upgrade/defUpgradeQueue.js
Outdated
| if (!this._upgrade) throw new Error('Upgrade queue is not set') | ||
| const message = this._constructMessage(definition) | ||
| await this._upgrade.queue(JSON.stringify(message)) | ||
| this.logger.debug('Queued for definition upgrade ', { |
There was a problem hiding this comment.
I've heard this referred to as recompute which I see in the code in other places. I don't see upgrade in other parts of the code. Is there a reason to name the queue and method upgrade instead of recompute.
There was a problem hiding this comment.
This recompute happens only when schema version is update/upgraded. I am open to renaming it if it improves the readability.
jeffmendoza
approved these changes
Dec 11, 2024
mpcen
approved these changes
Dec 11, 2024
-add logging to handle queuing failure -remove redundant JSON.stringify when queuing message (there is no impact to message decoding) -move queueHandler and defUpgrader to local scope in setup -minor refactoring to improve code reuse and readability -add more tests
qtomlinson
force-pushed
the
qt/definition_upgrade
branch
from
c3cdd04 to
0efd310
Compare
|
Upon deployment, settings to turn on queuing: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Performance Degradation During Schema Version Upgrade
During the process of upgrading the definition schema version, performance degradation has been observed. Currently, schema version updates are handled by recomputing the definition on the fly, which causes slower performance. These updates are critical for data correctness and to introduce the LicenseRef marker.
Proposed Solution:
PR Details:
This PR introduces two implementations for the upgrade handler:
Environment Variables:
versionCheckas the default.memoryas the default.HARVEST_AZBLOB_CONNECTION_STRINGis used for connection information.definitions-upgrade.