IGNITE-6324 Transactional cache data partially available after crash. #2961
Conversation
dgovorukhin
force-pushed
the
ignite-6324
branch
from
19cf0cd
to
cb1cd80
Compare
| * @param topVer Topology version. | ||
| * @return List of consistent ids. | ||
| */ | ||
| public List<Object> mapToConsistentId(AffinityTopologyVersion topVer) { |
There was a problem hiding this comment.
We should not calculate this list on-demand. Since the list is fixed on topologyVersion, this list should be moved to the DiscoCache
| if (preLocked == null) | ||
| fileLockHolder = new FileLockHolder(storeMgr.workDir().getPath(), kernalCtx, log); | ||
|
|
||
| persStoreMetrics.wal(cctx.wal()); | ||
|
|
||
| recoveryIo = new RecoveryIoImp(kernalCtx, log); |
There was a problem hiding this comment.
RecoveryIo looks redundant, we do not use such a pattern. Instead, we probably should add a recovery processor which will add this listener on startup and manager recovery future(s).
| * | ||
| */ | ||
| private void restoreUpdates(final RecoveryContext recCtx) throws IgniteCheckedException { | ||
| final Map<T2<Integer, Integer>, T2<Integer, Long>> partStates = new HashMap<>(); |
There was a problem hiding this comment.
Map of tuples may be too heap-consuming. Check if CachePartitionPartialCountersMap can be used or modified for this use-case.
| /** | ||
| * | ||
| */ | ||
| public interface IgniteRecoveryFuture<R> extends IgniteInternalFuture<R> { |
There was a problem hiding this comment.
I think having recovery future dependant on WAL iterator is abstraction leak. We should simply have an init() method on it, and it will construct the required iterator internally.
| try { | ||
| f.get(); | ||
|
|
||
| if (msg instanceof TxStateRequest) { |
There was a problem hiding this comment.
Split different message processing into different methods
| } | ||
| }); | ||
|
|
||
| recoveryIo.onNodeLeft(new CI1<String>() { |
There was a problem hiding this comment.
We usually place onNodeLeft callbacks on futures, this is another indication that RecoveryIo should be removed
| ).toString(); | ||
|
|
||
| handler.apply(constId, (Message)msg); | ||
| } |
There was a problem hiding this comment.
We should not silently drop a message. If a listener is supposed to be always installed before a first message is sent, we should at least print a warning.
| @@ -315,7 +315,7 @@ public RecordDataV1Serializer(GridCacheSharedContext cctx) { | |||
| long msb = in.readLong(); | |||
| long lsb = in.readLong(); | |||
| boolean hasPtr = in.readByte() != 0; | |||
| int idx = hasPtr ? in.readInt() : 0; | |||
| long idx = hasPtr ? in.readLong() : 0; | |||
There was a problem hiding this comment.
Does not this break compatibility?
| FileWALPointer pntCandidate = null; | ||
|
|
||
| for (IgniteInternalTx tx : txs) | ||
| if (tx instanceof IgniteTxAdapter) { |
There was a problem hiding this comment.
Do not use instanceof, just add a method to the IgniteInternalTx interface
| WALPointer ptr = txAdapter.prepPtr; | ||
|
|
||
| if (ptr != null && ptr instanceof FileWALPointer) { | ||
| FileWALPointer ptr0 = (FileWALPointer)ptr; |
There was a problem hiding this comment.
Also, do not cast to FileWALPointer here, add Comparable to wal pointer interface.
No description provided.