Piggy back archival uploading progress on heartbeat details #1963
Conversation
yycptt
changed the title
|
Also fixed #1959 |
yycptt
force-pushed
the
archival-heartbeat
branch
from
34e9cab to
ff66bfa
Compare
| currIteratorState, err := historyBlobIterator.GetState() | ||
| if err == nil { | ||
| progress.IteratorState = currIteratorState | ||
| progress.BlobPageToken = pageToken |
There was a problem hiding this comment.
It feels weird to me that BlobPageToken and UploadedBlobs can become out of sync. The way I am reading this code is that progress.BlobPageToken represents the highest blob page you have successfully uploaded so far. But in the case where historyBlobIterator.GetState() returns error you get a progress which has a blob in UploadedBlobs with a higher page than progress.BlobPageToken
There was a problem hiding this comment.
Updated, but I still feel a little bit weird as I think progress.UploadedBlobs should always reflect what has been uploaded. In the case when GetState() returns an error, the blob is still uploaded successfully, so I think progress should still be updated.
| archiverTestMetrics.On("IncCounter", metrics.ArchiverScope, metrics.ArchiverDeleteLocalSuccessCount).Once() | ||
| archiverTestLogger.On("Error", mock.Anything, mock.Anything).Once() | ||
|
|
||
| env := s.NewTestWorkflowEnvironment() | ||
| env.OnActivity(uploadHistoryActivityFnName, mock.Anything, mock.Anything).Return(workflow.NewTimeoutError(shared.TimeoutTypeStartToClose)) | ||
| env.OnActivity(deleteBlobActivityFnName, mock.Anything, mock.Anything).Return(nil) | ||
| env.OnActivity(uploadHistoryActivityFnName, mock.Anything, mock.Anything).Return(nil, workflow.NewTimeoutError(shared.TimeoutTypeStartToClose)) |
There was a problem hiding this comment.
Will add timeout details after client side test framework is fixed.
| currIteratorState, err := historyBlobIterator.GetState() | ||
| if err == nil { | ||
| progress.IteratorState = currIteratorState | ||
| progress.BlobPageToken = pageToken |
There was a problem hiding this comment.
Updated, but I still feel a little bit weird as I think progress.UploadedBlobs should always reflect what has been uploaded. In the case when GetState() returns an error, the blob is still uploaded successfully, so I think progress should still be updated.
#1536