Skip to content

Commit

Permalink
* move invocations toIPostSaver.Save() & UserSaver.Save() inside …
Browse files Browse the repository at this point in the history
…try block to ensure their `OnPostSave()` get invoked in finally block even if exception was thrown to fix 388ddc9 @ `CrawlFacade.SaveCrawled()`

* move the action returned by `Save()` into `IDisposable.Dispose()` to allow locks also get released by GC
* lift variable `(newly|already)LockedImages` from method `Save()` to nullable class field
@ ReplyContentImageSaver.cs
@ c#/crawler
  • Loading branch information
n0099 committed Jun 12, 2024
1 parent 18dd2c3 commit 55c529f
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 30 deletions.
17 changes: 9 additions & 8 deletions c#/crawler/src/Tieba/Crawl/Facade/CrawlFacade.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,21 @@ public virtual void Dispose()
using var transaction = db.Database.BeginTransaction(IsolationLevel.ReadCommitted);

var postSaver = postSaverFactory(Posts);
var savedPosts = Posts.IsEmpty ? null : postSaver.Save(db);

var userSaver = userSaverFactory(_users);
userSaver.Save(db,
postSaver.CurrentPostType,
postSaver.UserFieldUpdateIgnorance,
postSaver.UserFieldRevisionIgnorance);

OnBeforeCommitSave(db, userSaver);
try
{
var savedPosts = Posts.IsEmpty ? null : postSaver.Save(db);
userSaver.Save(db,
postSaver.CurrentPostType,
postSaver.UserFieldUpdateIgnorance,
postSaver.UserFieldRevisionIgnorance);

OnBeforeCommitSave(db, userSaver);

db.TimestampingEntities();
_ = db.SaveChanges();
transaction.Commit();

if (savedPosts != null) OnPostCommitSave(savedPosts, stoppingToken);
return savedPosts;
}
Expand Down
48 changes: 26 additions & 22 deletions c#/crawler/src/Tieba/Crawl/Saver/ReplyContentImageSaver.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,26 @@
namespace tbm.Crawler.Tieba.Crawl.Saver;

public class ReplyContentImageSaver(ILogger<ReplyContentImageSaver> logger)
public sealed class ReplyContentImageSaver(ILogger<ReplyContentImageSaver> logger) : IDisposable
{
private static readonly ConcurrentDictionary<string, ImageInReply>
GlobalLockedImagesInReplyKeyByUrlFilename = new();
private Dictionary<string, ImageInReply>? _newlyLockedImages;
private Dictionary<string, ImageInReply>? _alreadyLockedImages;

public void Dispose()
{
try
{
if (_newlyLockedImages != null && _newlyLockedImages.Any(pair =>
!GlobalLockedImagesInReplyKeyByUrlFilename.TryRemove(pair)))
throw new InvalidOperationException();

Check failure on line 16 in c#/crawler/src/Tieba/Crawl/Saver/ReplyContentImageSaver.cs

View workflow job for this annotation

GitHub Actions / build (crawler)

An exception is thrown from the Dispose() method in type ReplyContentImageSaver (https://github.com/Vannevelj/SharpSource/blob/master/docs/SS027-ExceptionThrownFromDispose.md)

Check failure on line 16 in c#/crawler/src/Tieba/Crawl/Saver/ReplyContentImageSaver.cs

View workflow job for this annotation

GitHub Actions / build (crawler)

Remove this 'throw' statement. (https://rules.sonarsource.com/csharp/RSPEC-3877)

Check failure on line 16 in c#/crawler/src/Tieba/Crawl/Saver/ReplyContentImageSaver.cs

View workflow job for this annotation

GitHub Actions / build (crawler)

An exception is thrown from the Dispose() method in type ReplyContentImageSaver (https://github.com/Vannevelj/SharpSource/blob/master/docs/SS027-ExceptionThrownFromDispose.md)

Check failure on line 16 in c#/crawler/src/Tieba/Crawl/Saver/ReplyContentImageSaver.cs

View workflow job for this annotation

GitHub Actions / build (crawler)

Remove this 'throw' statement. (https://rules.sonarsource.com/csharp/RSPEC-3877)
}
finally
{
_newlyLockedImages?.Values().ForEach(Monitor.Exit);
_alreadyLockedImages?.Values().ForEach(Monitor.Exit);
}
}

public Action Save(CrawlerDbContext db, IEnumerable<ReplyPost> replies)
{
Expand Down Expand Up @@ -36,28 +53,28 @@ where images.Keys.Contains(e.UrlFilename)
var newImages = images
.ExceptByKey(existingImages.Keys).ToDictionary();

var newlyLockedImages = newImages
_newlyLockedImages = newImages
.Where(pair => GlobalLockedImagesInReplyKeyByUrlFilename.TryAdd(pair.Key, pair.Value))
.ToDictionary();
newlyLockedImages.Values()
_newlyLockedImages.Values()
.Where(image => !Monitor.TryEnter(image, TimeSpan.FromSeconds(10)))
.ForEach(image => logger.LogWarning(
"Wait for locking newly locked image {} timed out after 10s", image.UrlFilename));

var alreadyLockedImages = GlobalLockedImagesInReplyKeyByUrlFilename
_alreadyLockedImages = GlobalLockedImagesInReplyKeyByUrlFilename
.IntersectByKey(newImages
.Keys().Except(newlyLockedImages.Keys()))
.Keys().Except(_newlyLockedImages.Keys()))
.ToDictionary();
alreadyLockedImages.Values()
_alreadyLockedImages.Values()
.Where(image => !Monitor.TryEnter(image, TimeSpan.FromSeconds(10)))
.ForEach(image => logger.LogWarning(
"Wait for locking already locked image {} timed out after 10s", image.UrlFilename));

if (alreadyLockedImages.Count != 0)
if (_alreadyLockedImages.Count != 0)
existingImages = existingImages
.Concat((
from e in db.ImageInReplies.AsTracking()
where alreadyLockedImages.Keys().Contains(e.UrlFilename)
where _alreadyLockedImages.Keys().Contains(e.UrlFilename)
select e).ToDictionary(e => e.UrlFilename))
.ToDictionary();
(from existing in existingImages.Values
Expand Down Expand Up @@ -88,19 +105,6 @@ on existing.UrlFilename equals replyContentImage.ImageInReply.UrlFilename
.ExceptBy(existingReplyContentImages.Select(e => (e.Pid, e.UrlFilename)),
e => (e.Pid, e.ImageInReply.UrlFilename)));

return () =>
{
try
{
if (newlyLockedImages.Any(pair =>
!GlobalLockedImagesInReplyKeyByUrlFilename.TryRemove(pair)))
throw new InvalidOperationException();
}
finally
{
newlyLockedImages.Values().ForEach(Monitor.Exit);
alreadyLockedImages.Values().ForEach(Monitor.Exit);
}
};
return Dispose;
}
}

0 comments on commit 55c529f

Please sign in to comment.