2017-03-08 8 views
1

SpringブートアプリケーションでJPA hibernateを使用してDBに永続化されるMeetingクラスがREST APIによってインターフェイスされています。スレッドセーフです。これは会議のクラスです:制限付き実行でのビジネスサービス操作の効率的なスレッドセーフ実装

@Entity 
public class Meeting { 
    @Id 
    @GeneratedValue(strategy= GenerationType.AUTO) 
    private Long id; 

    @ManyToOne(optional = false) 
    @JoinColumn(name = "account_id", nullable = false) 
    private Account account; 

    private Integer maxAttendees; 
    private Integer numAttendees; // current number of attendees 
    ... 
} 

私は、アカウントエンティティを持っていることがわかります。アカウントには多くの会議が関連付けられています。会議には最大参加者数があり、アカウントにはスケジュールされた会議の最大数があります。同様に、アカウントにはmaxSchedulesおよびnumSchedules変数があります。

基本的なワークフローは次のとおりです。会議が作成され、スケジュールされてから、出席者が個別に登録されます。

:ここでの主な目標は、許可される操作(スケジュールまたはレジスタ)の最大数を超えないようにすることです。

私は当初、当初のスケジュールと登録参加者のための私のビジネスサービスはこのように見えた、パフォーマンスよりもビジネスロジックに重点を置いた:

@Service 
public class MeetingService { 
    ... 

    @Transactional 
    public synchronized void scheduleMeeting(Long meetingId, Date meetingDate) { 
     Meeting meeting = repository.findById(meetingId); 
     Account account = meeting.getAccount(); 
     if(account.getNumSchedules() + 1 <= account.getMaxSchedules() 
      && meeting.getStatus() != SCHEDULED) { 
      meeting.setDate(meetingDate); 
      account.setNumSchedules(account.getNumSchedules()+1); 
      // save meeting and account here 
     } 

     else { throw new MaxSchedulesReachedException(); } 
    } 

    @Transactional 
    public synchronized void registerAttendee(Long meetingId, String name) { 
     Meeting meeting = repository.findById(meetingId); 
     if(meeting.getNumAttendees() + 1 <= meeting.getMaxAttendees() 
      && meeting.getStatus() == SCHEDULED) { 
      meeting.setDate(meetingDate); 
      meeting.setNumAttendees(account.getNumAttendees()+1); 
      repository.save(meeting); 
     } 

     else { throw new NoMoreAttendeesException(); } 
    } 
    ... 
} 

このアプローチの問題は、同期メソッドのロックがあるということですオブジェクト(this)の場合、サービスはシングルトンインスタンスなので、複数のスレッドが2つの同期化された操作のいずれかを実行しようとしているときには、ロックが解放されるのを待つ必要があります。登録したいスレッドがブロックしてはならないことを意味し、私は問題をブロック間の操作を解いこれにより

... 
private final Object scheduleLock = new Object(); 
private final Object registerLock = new Object(); 
... 

@Transactional 
public void scheduleMeeting(Long meetingId, Date meetingDate) { 
    synchronized (scheduleLock) { 
     Meeting meeting = repository.findById(meetingId); 
     Account account = meeting.getAccount(); 

     if(account.getNumSchedules() + 1 <= account.getMaxSchedules() 
      && meeting.getStatus() != SCHEDULED) { 
      meeting.setDate(meetingDate); 
      account.setNumSchedules(account.getNumSchedules()+1); 
      // save meeting and account here 
     } 

     else { throw new MaxSchedulesReachedException(); } 
    } 
} 

@Transactional 
public void registerAttendee(Long meetingId, String name) { 
    synchronized (registerLock) { 
     Meeting meeting = repository.findById(meetingId); 
     if(meeting.getNumAttendees() + 1 <= meeting.getMaxAttendees() 
      && meeting.getStatus() == SCHEDULED) { 
      meeting.setDate(meetingDate); 
      meeting.setNumAttendees(account.getNumAttendees()+1); 
      repository.save(meeting); 
     } 

     else { throw new NoMoreAttendeesException(); } 
    } 
} 
... 

、:

は、私は、スケジューリング、登録のための分離ロックを使用して第二のアプローチに付属していますスケジューリング中のスレッドによって実行されます。

ここで問題となるのは、あるアカウントで会議のスケジュールを設定しているスレッドは、別のアカウントから会議をスケジュールしようとするスレッドによってロックされてはならないということです。同じアカウントで

私はまだ実装されていないが、アイデアはロックプロバイダを持つことである、のようなもののデザインに付属の、という固定の場合:

@Component 
public class LockProvider { 
    private final ConcurrentMap<String, Object> lockMap = new ConcurrentHashMap(); 

    private Object addAccountLock(Long accountId) { 
     String key = makeAccountKey(accountId); 
     Object candidate = new Object(); 
     Object existing = lockMap.putIfAbsent(key, candidate); 
     return (existing != null ? existing : candidate); 
    } 

    private Object addMeetingLock(Long accountId, Long meetingId) { 
     String key = makeMeetingKey(accountId, meetingId); 
     Object candidate = new Object(); 
     Object existing = lockMap.putIfAbsent(key, candidate); 
     return (existing != null ? existing : candidate); 
    } 

    private String makeAccountKey(Long accountId) { 
     return "acc"+accountId.toString(); 
    } 

    private String makeMeetingKey(Long accountId, Long meetingId) { 
     return "meet"+accountId.toString()+meetingId.toString(); 
    } 

    public Object getAccountLock(Long accountId) { 
     return addAccountLock(accountId); 
    } 

    public Object getMeetingLock(Long accountId, Long meetingId) { 
     return addMeetingLock(accountId, meetingId); 
    } 
} 

しかし、このアプローチは、維持することに余分な作業の多くが含まれますアカウント、ミーティングが削除されたときに使用されなくなったロックを破棄したり、それ以上の同期操作を実行できない状態になったりすることを確実にします。

それを実装する価値があるのか​​、それを行うより効率的な方法があるのか​​という疑問があります。

+0

「サービス」はなぜシングルトンである必要がありますか? –

+0

これはSpring Beanなので、デフォルトではシングルトンなので、フレームワークをより効率的にすることができます。特に、並行サービスの実行数が多い場合に、インスタンスを再利用するシングルトンでも同様のサービスが接続されているためです。しかしそれ以外にも、たとえサービスがシングルトンでなくても毎回新しいインスタンスがあったとしても、その問題をどのように解決できるかはわかりません。 – raspacorp

+0

あなたの同期のすべてに欠陥があります。サービスのメソッドを同期する必要はありません。さまざまな会議を同時にスケジュールすることができます(また、非常に望ましい)ので、サービスの方法を同期する必要はありません。 atendeesの登録と同じです。異なるアテンダントが同時に異なる会議に登録してもらいたいです。同期化されたメソッドはこれを正確に回避します。理想的には楽観的なロック機構(つまり、デフォルトではHibernateが実装しています)を使用して、これらのものをすべて削除し、データベースを代わりに決定させます。 –

答えて

2

おそらく、これは部分的にドメインロジックの問題です。

MeetingAccountへの実際の参照せずに作成することができないことを考えると、私はここにあなたのビジネスロジックのわずかな変化をお勧めします:

@Transactional 
public void scheduleMeeting(MeetingDTO meetingDto) { 
    // load the account 
    // force increment the version at commit time. 
    // this is useful because its our sync object but we don't intend to modify it. 
    final Account account = accountRepository.findOne( 
     meetingDto.getAccountId(), 
     LockMode.OPTIMISTIC_FORCE_INCREMENT 
); 

    if (account.getMeetingCount() > account.getMaxMeetings()) { 
    // throw exception here 
    } 

    // saves the meeting, associated to the referenced account. 
    final Meeting meeting = MeetingBuilder.from(meetingDto); 
    meeting.setAccount(account);  
    meetingRepository.save(meeting); 
} 

だからそのコードは正確に何をするのでしょうか?

  1. フェッチを使用してOPTIMISTIC_FORCE_INCREMENT

    これは基本的にJPAプロバイダに、トランザクションの最後に、Accountの更新文を発行して@Versionフィールドを1つインクリメントする必要があることを伝えます。

    これはトランザクションをコミットする最初のスレッドが勝つように強制するもので、残りのスレッドは当事者の遅れとみなされるため、OptimisticLockExceptionがスローされるためにロールバックする以外に選択肢はありません。

  2. 最大会議サイズが満たされていないことを確認します。もしそうなら、私たちは例外をスローします。

    私はおそらく#getMeetingCount()Accountではなく、コレクションを取得するに頼ると関連する会議の数を計算するために@Formulaを使用する場合がありますことを、ここで説明します。これにより、実際にはを変更する必要はありません。これは、Accountのいずれかが動作するために必要です。

  3. Accountに関連付けられた新しいMeetingを保存します。

    ここでの関係は一方向であると仮定して、保存する前に会議にAccountを関連付けます。

なぜここで同期のセマンティクスは必要ないのですか?

すべてが(1)に戻ってきます。基本的に最初にコミットするトランザクションが成功すると、OptimisticLockExceptionが返され、の場合は、複数のスレッドが同じアカウントに関連付けられた会議を同時にスケジュールしようとします。

#scheduleMeetingへの2回のコールが、トランザクションが重複しないところで連続して発生する場合、問題は発生しません。

このソリューションは、データベースまたはカーネルモードのロックを使用しないため、他のソリューションを実行しません。

+0

見た目はいいですが、特定のアカウントや会議だけを選択できるのはまだわかりませんが、フェッチ時にバージョンの増分を強制していますが、フィールドに注釈を付けるときは@Version任意のアップデートを取ってバージョンを増やすでしょうか?私は2つ以上の操作を持っていますが、私はスケジュールだけを表示し、同期したものを登録していますが、別の操作が同じアカウント内の他のフィールドを更新している場合、スケジュール操作が失敗することはありません。現在のスケジュール数。 – raspacorp

+1

ここでデータの正規化を適用します。 'Account'への' @ OneToOne'である 'AccountMeetingDetails'を作成し、そのアカウントに関連付けられた' @ Version'と会議用のものを追加します。この方法で、他のビジネスドメインの事柄に影響を与えることなくそのバージョンを増やすことができます。 – Naros

関連する問題