Story/perf trace handle multiline queries #40
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 prefetch mechanism is not sufficient to guarantee that all queries
are performed in bulk. For example, consider the code:
for rec in batch:
print(rec.order_id.partner_id.name)
Since this code uses only iteration and field traversal by attribute
access, the same prefetch object will be used throughout.
On the first iteration, evaluating "rec.order_id" should therefore
fetch the field "order_id" (along with all other prefetchable fields)
for all records in "batch", and add all of the "order_id" record IDs
to the prefetch object.
Similarly, evaluating ".partner_id" should fetch the field
"partner_id" (along with all other prefetchable fields) for all
applicable "order_id" records, and add all of the "partner_id" record
IDs to the prefetch object.
Finally, evaluating ".name" should fetch the field "name" (along with
all other prefetchable fields) for all applicable "partner_id"
records.
All fetches are therefore performed as bulk operations on the first
iteration of the loop, and all subsequent loop iterations therefore
experience a 100% cache hit rate.
A problem occurs if some of the intermediate "order_id" or
"partner_id" values are already present in the cache. On the first
iteration, if "rec.order_id" is already present in the cache then
evaluating "rec.order_id" will just return the cached value, and will
add only this "order_id" record ID to the prefetch object.
Subsequently evaluating ".partner_id" will fetch fields for only the
single "order_id" record, since that is the only record present in the
prefetch object.
The net result is that code expected to exploit the prefetch mechanism
to perform bulk fetch operations may end up degenerating to perform
multiple single-record fetch operations. This degeneration is
unfortunately dependent on the a priori cache state and so cannot
viably be predicted.
The prefetch mechanism must therefore be seen as a heuristic that will
attempt to perform bulk operations but cannot guarantee to do so, even
with careful management of the prefetch object.
Revert therefore to using explicit precaching, with sensible defaults
and well-defined injection points for derived models.
Note that the ".product_tmpl_id.name" is not strictly necessary, since
the same effect could be achieved via just ".name". The explicit
"product_tmpl_id" is included to make it easier for the reader to
understand why this extra precache fetch is required.
Signed-off-by: Michael Brown <mbrown@fensystems.co.uk>
User-story: 3101 Signed-off-by: Michele Costa <michele.costa@unipart.io>
nocturnalastro
force-pushed
the
story/perf-trace_2
branch
from
49a4c91
to
dbb2a32
Compare
Allow a gateway to be configured to require manual processing of EDI documents. Signed-off-by: Michael Brown <mbrown@fensystems.co.uk>
Task: 4371 Signed-off-by: Miquel PS <miquel.palahi.sitges@unipart.io>
Task: 4373 Signed-off-by: Miquel PS <miquel.palahi.sitges@unipart.io>
A synchronizer will not recognise a change in an edi related field, if that field is not set beforehand. By marking the field as "target not yet known" this signals to the synchronizer that this value will be a new record after execution and therefore should not be elided. User-story: story/4349 Signed-off-by: Michele Costa <michele.costa@unipart.io>
The EDI gateway base test case asserts that no issues were raised as part of tearDown(), but does so before calling the superclass tearDown() method. If this assertion fails, the superclass method will not be called and so the transaction will never be rolled back, resulting in the failure of all remaining tests in the suite. Fix by delaying the assertion until after calling the superclass. Story: 4392 Signed-off-by: Michael Brown <mbrown@fensystems.co.uk>
If a user had permission to modify the paths of the local filesystem gateway they were able to retrieve files from the server, provided the user running the server has access to them. This change adds a restriction to local filesystem connections that they can only access files within a pre-defined path. Changing that path requires modifying the config file and restarting the service. User-story: story/4114 Signed-off-by: Michael Brown <mbrown@fensystems.co.uk>
The EDI sale base test case defines three test products with a type of "product" (i.e. stockable product, rather than consumable). This type is defined only by the "stock" module, which is deliberately not a dependency of the "edi_sale" module. The EDI sale test cases do not require the products to be stockable, so fix by simply omitting the product type when creating the product. This problem is not detected by the standard CI test run since the standard test setup installs all EDI modules including "edi_stock", which drags in the "stock" module and so hides the problem. Reported-by: Luke Barone-Adesi <luke.barone-adesi@unipart.io> Reported-by: Przemysław Buczkowski <pbuczkowski@unipart.io> Signed-off-by: Michael Brown <mbrown@fensystems.co.uk>
Story/4226 Task/4774 Signed-off-by: Kevin Dwyer <kevin.dwyer@unipart.io>
This test executes a document with a patch that causes failure in order to test the rollback functionality.
Clearing the cache and fields that need to be recalculated after a failure rollback will prevent further failures from recalculations or accessing cached objects
Recent changes to fix issues with comparators has introduced a
new edge case. Two identical values may be evaluated as different
when part of them (while being the same value) needs to be
constructed as another new record. Example: partner title record
has not yet been constructed and the same partner is attempted to
be created twice, this results in the partner being identified as
different causing an attempt at creating the parnter twice, the
unique constraint for a partner then causes the creation of a
partner to be invalidated.
Signed-off-by: Robert Smith <robert.smith@unipart.io>
When a partner is created and no title of the type already exists a models.NewId is used, this causes the same person ingested twice to be incorrectly treated as separate entities. Signed-off-by: Robert Smith <robert.smith@unipart.io>
Signed-off-by: David Spence <david.spence@unipart.io>
Signed-off-by: Michele Costa <michele.costa@unipart.io>
nocturnalastro
force-pushed
the
story/perf-trace_2
branch
from
dbb2a32
to
f05cd05
Compare
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.
Signed-off-by: Michele Costa michele.costa@unipart.io