1

アーカイブのように、古いファイルが保存されているリポジトリがあります。
ユーザーは単純な Web アプリケーションを使用してこれらのファイルを取得します。
Web アプリが実行されているサーバーで単純なファイル システム キャッシュを維持しています。
少なくともそれは単なるアイデアでしたが、シンプルに見えました:)

一度に 1 つのスレッドのみがアーカイブから同じファイルをフェッチできるように、そのキャッシュでファイルの作成を同期する必要があります。

そのファイルをたまたま必要とする他のすべてのスレッドは、最初のスレッドがキャッシュへの書き込みを終了するまで待機し、そこからフェッチする必要があります。
最初は File.exists() メソッドを使用しましたが、スレッド (ロック所有者) が空のファイルを作成した直後に true を返すため (リポジトリ ストリームから書き込みを開始できるようにするため)、これは良くありません。

それが正しい方法かどうかはわかりませんが、静的マップ (file_ID を syncDummyObject にマップする) を使用して、現在フェッチされているファイルを追跡しています。
次に、そのsyncDummyObjectでファイルフェッチを同期しようとしました。

これはこれを行う正しい方法ですか?コードは機能していますが、本番環境に置く前に、問題がないことを確認する必要があります。

ファイルを作成し、完了したらキャッシュに転送するステージングディレクトリを使用することを検討しましたが、別の問題が発生する可能性があります...

読みやすくするために、エラー処理のログと無関係な部分を削除しました。

ありがとう!

public class RepoFileFetcher{

    private static volatile ConcurrentHashMap<String, Object> syncStrings = new ConcurrentHashMap<String, Object>();    
    private static final Object mapSync = new Object(); // map access sync
    private Boolean isFileBeingCreated = new Boolean(false);
    private Boolean isFileReadyInCache = new Boolean(false);


    public File getFileById(MikFileIdentifier cxfi){        
        File theFile = null; // file I'm going to return in the end

        try{
            Object syncObject = null;

            // sync map access
            synchronized(mapSync){

                if(syncStrings.containsKey(cxfi.getFilePath())){
                    // if the key exists in the map it means that
                    // it's being created by another thread
                    // fetch the object from the map 
                    // and use it to wait until file is created in cache
                    syncObject = syncStrings.get(cxfi.getFilePath());

                    isFileBeingCreated = true;


                }else if(!(new File(cxfi.getFilePath())).exists()){
                    // if it doesn't exist in map nor in cache it means that
                    // I'm the first one that fetches it from repo
                    // create new dummyLockObject and put it in the map
                    syncObject = new Object();
                    syncStrings.put(cxfi.getFilePath(), syncObject);

                }else{
                    // if it's not being created and exists in cache
                    // set flag so I can fetch if from the cache bellow
                    isFileReadyInCache = true;
                }
            }


            // potential problem that I'm splitting the critical section in half,
            // but I don't know how to avoid locking the whole fetching process
            // I want to lock only on the file that's being fetched, not fetching of all files (which I'd get if the mapSync was still locked)
            // What if, at this very moment, some other thread starts fetching the file and isFileBeingCreated becomes stale? Is it enough to check whether I succeeded renaming it and if not then fetch from cache? 


            if(!isFileBeingCreated && !isFileReadyInCache){

                // skip fetching from repo if another thread is currently fetching it
                // sync only on that file's map object

                synchronized(syncObject){
                    File pFile = new File(cxfi.getFilePath());
                    pFile.createNewFile();

                    // ...
                    // ... the part where I write to pFile from repo stream
                    // ...

                    if(!pFile.renameTo(theFile)){
                        // file is created by someone else 
                        // fetch it from cache
                        theFile = fetchFromCache(cxfi, syncObject);
                    }

                    syncStrings.remove(cxfi.getFilePath());

                    // notify all threads in queue that the file creation is over
                    syncObject.notifyAll();
                }//sync

            }else{

                theFile = fetchFromCache(cxfi, syncObject);
            }

            return theFile;


        }catch(...{
            // removed for better readability
        }finally{
            // remove from the map, otherwise I'll lock that file indefinitely
            syncStrings.remove(cxfi.getFilePath());
        }

        return null;
    }


    /**
     * Fetches the file from cache
     * @param cxfi File identification object
     * @param syncObject Used to obtain lock on file
     * @return File from cache
     * @throws MikFileSynchronizationException
     * @author mbonaci
     */
    private File fetchFromCache(FileIdentifier cxfi, Object syncObject)
            throws MikFileSynchronizationException{

        try{
            // wait till lock owner finishes creating the file
            // then fetch it from the cache

            synchronized(syncObject){   

                // wait until lock owner removes dummyObject from the map
                // while(syncStrings.containsKey(cxfi.getFilePath()))
                // syncObject.wait();                   

                File existingFile = new File(cxfi.getFilePath());
                if(existingFile.exists()){
                    return existingFile;
                }else{
                    // this should never happen
                    throw new MikFileSynchronizationException();
                }
            }

        }catch(InterruptedException ie){
            logger.error("Synchronization error", ie);
        }
        return null;
    }

編集I: ご協力いただきありがとうございます。CHM で putIfAbsent() を使用するという提案が鍵でした。私は最終的にこのようにしました(追加のコメントは大歓迎です):

編集 II:クラス の他のブランチに CHM 要素の削除を追加しましifた (必要がない場合でも、要素をマップに配置するようになりました)。

編集 III: 上記の変数のファイル存在のチェックを移動しましたisFileInCache

public class RepoFileFetcher{               
    private static volatile ConcurrentHashMap<String, Object> syncStrings = new ConcurrentHashMap<String, Object>();    

    // save some time so I can lock syncObject earlier 
    private boolean isFileInCache = false;

    // remember whether we put the elem in the map or not 
    // so we know whether to remove it later 
    private boolean insertedMapElem = false; // added in EDIT II 

    /**
     * Fetches the file from repository (anc caches it) or directly from cache if available
     * @param cxfi File identification object
     * @return File
     * @author mbonaci
     */
    public File getFileById(FileIdentifier cxfi){
        String fileId = cxfi.getFileId();
        String fileName = cxfi.getOnlyPath() + fileId;
        File theFile = null; // file I'm going to return in the end

        try{
            Object syncObject = null;
            Object dummyObject = new Object();                  
            isFileInCache = (new File(fileName)).exists();
            syncObject = syncStrings.putIfAbsent(fileId, dummyObject);

            if(syncObject == null){ // wasn't in the map                            
                insertedMapElem = true; // we put the new object in

                if(!isFileInCache){ // not in cache

                    // if it doesn't exist in map nor in cache it means that
                    // I'm the first one that fetches it from repo (or cache was deleted)
                    // syncObject = new lock object I placed in the map
                    syncObject = dummyObject;

                    synchronized(syncObject){
                        File pFile = new File(cxfi.getFilePath());
                        pFile.createNewFile();

                        // ...
                        // ... the part where I write to pFile from repo stream
                        // ...

                        pFile.renameTo(theFile)
                        theFile = pFile;

                        syncStrings.remove(cxfi.getFilePath());

                        // notify all threads in queue that the file is now ready to be fetched from cache
                        syncObject.notifyAll();
                    }//sync

                }else{
                    // if it's not being created and exists in cache it means that it's complete
                    // fetch it from cache without blocking (only reading)
                    syncStrings.remove(cxfi.getFilePath()); // added in EDIT II
                    theFile = new File(fileName);
                }

            }else{
                // if the key exists in the map it means that
                // it's being created by another thread
                // fetch the object from the map 
                // and use it to wait until file is created in cache
                // don't touch the map (I haven't added anything)
                // the lock owner will remove the elem
                // syncObject = the object that what was in the map when I called putIfAbsent()
                theFile = fetchFromCache(cxfi, syncObject);
            }

            return theFile;

        }catch(...{
            // removed for better readability
        }finally{ 
            // no good cuz' this way I'd sometimes remove the map elem
            // while lock owner still writes to a file
            // only the one who placed the elem in the map should remove it
            // remove from the map, otherwise I'll lock that file indefinitely
            // syncStrings.remove(fileId); // commented out in EDIT II
        }
        // remove in case of exception (but only if we added it)
        if(insertedMapElem)
            syncStrings.remove(fileId);
        return null;
    }


    /**
     * Fetches the file from cache after it obtains lock on <code>syncObject</code>
     * @param cxfi File identification object
     * @param syncObject Used to obtain lock on file
     * @return File from cache
     * @author mbonaci
     */
    private File fetchFromCache(FileIdentifier cxfi, Object syncObject){
        String fileId = cxfi.getFileId();
        String fileName =  fileId + ".tif";

        synchronized(syncObject){

            File existingFile = new File(cxfi.getAbsPath() + fileName);

            if(existingFile.exists()){
                return existingFile;
            }
        }
    }
4

1 に答える 1

3

いくつかの微調整を提案できますか:

  1. をすでに使用しているConcurrentHashMapため、追加のロックは必要ありません。
  2. 「ファイル」を、独自の同期を持つよりスマートなオブジェクトにラップします。したがって、次のようなことができます:

    • putIfAbsent()パスとファイルをラップする「スマート」オブジェクトを使用してマップ上で呼び出します。
    • 上記は値を返します(パスが存在しない場合は新しい値、または既存のラッパーのいずれか)
    • すでにキャッシュされているかどうかを知るラッパーの状態を持っている
    • cache()キャッシュされているかどうかを確認し、キャッシュされている場合は何もせず、そうでない場合はキャッシュする呼び出し
    • 次に、ラッパーから「ファイル」を返します(getFile()メソッドなど)

次に、パブリック関数のラッパー内でロックを使用していることを確認します。これは、cache()同時に発生している間ブロックすることを意味します。

ここにスケッチがあります:

class CachedFile
{
  File realFile;
  // Initially not cached
  boolean cached = false;

  // Construct with file

  public synchronized boolean isCached()
  { return cached; }

  public synchronized void cache()
  {
    if (!cached)
    {
      // now load - safe in the knowledge that no one can get the file (or cache())
      ..
      cached = true; // done
    }
  }

  public synchronized <File> getFile()
  {
    // return the "file"
  }
}

コードは次のようになります。

ConcurrentHashMap<String, CachedFile> myCache = ConcurrentHashMap<>();
CachedFile newFile = new CachedFile(<path>);
CachedFile file = myCache.putIfAbsent(<path>, newFile);
// Use the new file if it did not exist
if (file == null) file = newFile;
// This will be no-op if already cached, or will block is someone is caching this file.
file.cache();
// Now return the cached file.
return file.getFile();

私の提案は理にかなっていますか?

于 2012-11-07T11:26:53.613 に答える