Bug #5281
BUFFER-COPY does not report changes in some modes
0%
History
#1 Updated by Eric Faulhaber about 3 years ago
- Project changed from Regression Testing to Database
#2 Updated by Eric Faulhaber about 3 years ago
When any change is made to a record, RecordBuffer
must invoke reportChange
to notify registered listeners of the change via the ChangeBroker
. This occurs in the RecordBuffer
invocation handler code (for individual property updates) or during endBatch
processing (for property updates collected in a batch assign or in a buffer-copy operation).
We recently introduced two optimizations which need to do this notification as well: the separate, fast path buffer-copy implementation (RecordBuffer.fastCopy
), and the direct access mode within the "classic" buffer-copy implementation.
Adrian posted the following in a separate task, while we were working a related issue:
Eric Faulhaber wrote:
But there is more... In
RecordBuffer.fastCopy
, we are not collecting the unreported changes and then reporting them after the batch copy is finished. We are setting and unsetting thebulkCopy
flag in that method, but that is only useful for the invocation handler path, which we are bypassing infastCopy
. We need a "bulk" way to collect the unreported changes (i.e., pairs of before/after field diffs), or we may have other problems resulting from going down thefastCopy
path. It looks like this would need to be collected inBaseRecord.setAllDatum
(which we probably should rename tosetAllData
).Right, a fast-copy won't report the changes. However, this means that the "regular" copy
RecordBuffer.copy(DataModelObject,DataModelObject,Map<DatumAccess,DatumAccess>)
also has the same issue. Some fields are updated usingOrmUtils.setField()
, but those changes aren't reported. We should find a way to fix both.
- For the "one-by-one" buffer-copy, we can simply use
BaseRecord.getActiveUpdateDiffs()
andRecordBuffer.addUnreportedChanges()
.- For the fast-copy, I agree with you.
BaseRecord.setAllDatum()
collects them, but the reporting should be done byRecordBuffer.fastCopy()
.To be fixed: right now I see that setter used inside
RecordBuffer$Handler.invoke()
can invalidateactiveBuffer
(currentRecord.setActiveBuffer(null);
). This can be dangerous forRecordBuffer.copy
asactiveBuffer
can be invalidated right in the middle of the buffer copy.
#3 Updated by Eric Faulhaber about 3 years ago
Adrian Lungu wrote:
To be fixed: right now I see that setter used inside
RecordBuffer$Handler.invoke()
can invalidateactiveBuffer
(currentRecord.setActiveBuffer(null);
). This can be dangerous forRecordBuffer.copy
asactiveBuffer
can be invalidated right in the middle of the buffer copy.
I wrote setActiveBuffer
with the intention of only using it from the invocation handler, so the invocation handler set it and unset it by design. BaseRecord.setDatum
was the only method that was supposed to have a dependency on activeBuffer
, and setDatum
was only meant to be called by DMO setter methods. It was not possible to pass the buffer as a parameter to setDatum
, because this method is called from the subclass DMO's setter methods which have no knowledge of RecordBuffer
. I tried to express this restriction in the javadoc, but I was not clear enough. When a call to setDatum
from OrmUtils.setField
was later added, this fragile requirement was broken. The use of activeBuffer
has since been expanded beyond this original intent.