Skip to content

Commit a68515d

Browse files
jfantinhardestyfoodprocessor
jfantinhardesty
and
foodprocessor
authored
Fix race conditions block cache (#566)
* Add support for GetCommittedBlocklist on S3storage * Fix block cache with S3 storage * Uncomment block cache tests for s3 * lint * Fix data races * Fix go mod * Revert "Fix data races" This reverts commit a10097b. * Don't run block cache tests on Windows * Add locks in block cache to prevent race conditions on Windows Co-authored-by: Michael Habinsky <foodprocessor@users.noreply.github.com> * Enable block cache tests in CI/CD * Fix deadlock * Fix issues with append * Try to fix issue with block cache temp dir in CI/CD --------- Signed-off-by: James Fantin-Hardesty <24646452+jfantinhardesty@users.noreply.github.com> Co-authored-by: Michael Habinsky <foodprocessor@users.noreply.github.com>
1 parent 9a739ea commit a68515d

File tree

3 files changed

+105
-93
lines changed

3 files changed

+105
-93
lines changed

‎.github/workflows/code-coverage.yml‎

Lines changed: 91 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ jobs:
3333
env:
3434
MOUNT_DIR: "$HOME/blob_mnt"
3535
TEMP_DIR: "$HOME/cloudfuse_tmp"
36+
BLOCK_TEMP_DIR: "$HOME/cloudfuse_block_tmp"
3637
WORK_DIR: "."
3738
cloudfuse_ADLS_CFG: "./cloudfuse.adls.yaml"
3839
cloudfuse_CFG: "./cloudfuse.yaml"
@@ -180,29 +181,29 @@ jobs:
180181
sudo fusermount -u ${{ env.MOUNT_DIR }}
181182
sleep 5
182183
183-
# - name: Create Config File - Block Blob - Block Cache
184-
# env:
185-
# NIGHTLY_STO_ACC_NAME: "${{ secrets.NIGHTLY_STO_BLOB_ACC_NAME }}"
186-
# NIGHTLY_STO_ACC_KEY: "${{ secrets.NIGHTLY_STO_BLOB_ACC_KEY }}"
187-
# ACCOUNT_TYPE: block
188-
# ACCOUNT_ENDPOINT: https://${{ secrets.NIGHTLY_STO_BLOB_ACC_NAME }}.blob.core.windows.net
189-
# VERBOSE_LOG: false
190-
# USE_HTTP: false
191-
# run: "./cloudfuse.test -test.v -test.coverprofile=${{ env.WORK_DIR }}/cloudfuse_gentest1.cov gen-test-config --config-file=azure_key_block_cache.yaml --container-name=${{ matrix.containerName }} --temp-path=${{ env.TEMP_DIR }} --output-file=${{ env.cloudfuse_CFG }}"
192-
193-
# - name: Block Blob Coverage - Block Cache
194-
# run: |-
195-
# rm -rf ${{ env.MOUNT_DIR }}/*
196-
# rm -rf ${{ env.TEMP_DIR }}/*
197-
# ./cloudfuse.test -test.v -test.coverprofile=${{ env.WORK_DIR }}/cloudfuse_block_block_cache.cov mount ${{ env.MOUNT_DIR }} --config-file=${{ env.cloudfuse_CFG }} --foreground=true &
198-
# sleep 10
199-
# ps -aux | grep cloudfuse
200-
# rm -rf ${{ env.MOUNT_DIR }}/*
201-
# cd test/e2e_tests
202-
# go test -v -timeout=7200s ./... -args -mnt-path=${{ env.MOUNT_DIR }} -tmp-path=${{ env.TEMP_DIR }}
203-
# cd -
204-
# sudo fusermount -u ${{ env.MOUNT_DIR }}
205-
# sleep 5
184+
- name: Create Config File - Block Blob - Block Cache
185+
env:
186+
NIGHTLY_STO_ACC_NAME: "${{ secrets.NIGHTLY_STO_BLOB_ACC_NAME }}"
187+
NIGHTLY_STO_ACC_KEY: "${{ secrets.NIGHTLY_STO_BLOB_ACC_KEY }}"
188+
ACCOUNT_TYPE: block
189+
ACCOUNT_ENDPOINT: https://${{ secrets.NIGHTLY_STO_BLOB_ACC_NAME }}.blob.core.windows.net
190+
VERBOSE_LOG: false
191+
USE_HTTP: false
192+
run: "./cloudfuse.test -test.v -test.coverprofile=${{ env.WORK_DIR }}/cloudfuse_gentest1.cov gen-test-config --config-file=azure_key_block_cache.yaml --container-name=${{ matrix.containerName }} --temp-path=${{ env.BLOCK_TEMP_DIR }} --output-file=${{ env.cloudfuse_CFG }}"
193+
194+
- name: Block Blob Coverage - Block Cache
195+
run: |-
196+
rm -rf ${{ env.MOUNT_DIR }}/*
197+
rm -rf ${{ env.BLOCK_TEMP_DIR }}/*
198+
./cloudfuse.test -test.v -test.coverprofile=${{ env.WORK_DIR }}/cloudfuse_block_block_cache.cov mount ${{ env.MOUNT_DIR }} --config-file=${{ env.cloudfuse_CFG }} --foreground=true &
199+
sleep 10
200+
ps -aux | grep cloudfuse
201+
rm -rf ${{ env.MOUNT_DIR }}/*
202+
cd test/e2e_tests
203+
go test -v -timeout=7200s ./... -args -mnt-path=${{ env.MOUNT_DIR }} -tmp-path=${{ env.BLOCK_TEMP_DIR }}
204+
cd -
205+
sudo fusermount -u ${{ env.MOUNT_DIR }}
206+
sleep 5
206207
207208
- name: Create Config File - ADLS
208209
env:
@@ -301,28 +302,27 @@ jobs:
301302
sudo fusermount -u ${{ env.MOUNT_DIR }}
302303
sleep 5
303304
304-
# - name: Create Config File - S3 - Block Cache
305-
# env:
306-
# S3_BUCKET_NAME: "${{ secrets.S3TEST_BUCKET_NAME }}"
307-
# S3_ENDPOINT: "${{ secrets.S3TEST_ENDPOINT }}"
308-
# S3_KEY_ID: "${{ secrets.S3TEST_ACCESS_KEY }}"
309-
# S3_REGION: "${{ secrets.S3TEST_REGION }}"
310-
# S3_SECRET_KEY: "${{ secrets.S3TEST_SECRET_KEY }}"
311-
# run: "./cloudfuse.test gen-test-config --config-file=s3_key_block_cache.yaml --temp-path=${{ env.TEMP_DIR }} --output-file=${{ env.cloudfuse_BLOCK_CFG }}"
305+
- name: Create Config File - S3 - Block Cache
306+
env:
307+
S3_BUCKET_NAME: "${{ secrets.S3TEST_BUCKET_NAME }}"
308+
S3_ENDPOINT: "${{ secrets.S3TEST_ENDPOINT }}"
309+
S3_KEY_ID: "${{ secrets.S3TEST_ACCESS_KEY }}"
310+
S3_REGION: "${{ secrets.S3TEST_REGION }}"
311+
S3_SECRET_KEY: "${{ secrets.S3TEST_SECRET_KEY }}"
312+
run: "./cloudfuse.test gen-test-config --config-file=s3_key_block_cache.yaml --temp-path=${{ env.BLOCK_TEMP_DIR }} --output-file=${{ env.cloudfuse_CFG }}"
312313

313-
# - name: S3 Coverage - Block Cache
314-
# run: |-
315-
# rm -rf ${{ env.MOUNT_DIR }}/*
316-
# rm -rf ${{ env.TEMP_DIR }}/*
317-
# sleep 1
318-
# ./cloudfuse.test -test.v -test.coverprofile=${{ env.WORK_DIR }}/cloudfuse_s3_block_cache.cov mount ${{ env.MOUNT_DIR }} --config-file=${{ env.cloudfuse_BLOCK_CFG }} --foreground=true &
319-
# sleep 10
320-
# ps -aux | grep cloudfuse
321-
# cd test/e2e_tests
322-
# go test -v -timeout=7200s ./... -args -mnt-path=${{ env.MOUNT_DIR }} -tmp-path=${{ env.TEMP_DIR }}
323-
# cd -
324-
# sudo fusermount -u ${{ env.MOUNT_DIR }}
325-
# sleep 5
314+
- name: S3 Coverage - Block Cache
315+
run: |-
316+
rm -rf ${{ env.MOUNT_DIR }}/*
317+
rm -rf ${{ env.BLOCK_TEMP_DIR }}/*
318+
./cloudfuse.test -test.v -test.coverprofile=${{ env.WORK_DIR }}/cloudfuse_s3_block_cache.cov mount ${{ env.MOUNT_DIR }} --config-file=${{ env.cloudfuse_CFG }} --foreground=true &
319+
sleep 10
320+
ps -aux | grep cloudfuse
321+
cd test/e2e_tests
322+
go test -v -timeout=7200s ./... -args -mnt-path=${{ env.MOUNT_DIR }} -tmp-path=${{ env.BLOCK_TEMP_DIR }}
323+
cd -
324+
sudo fusermount -u ${{ env.MOUNT_DIR }}
325+
sleep 5
326326
327327
- name: Create Config File - S3 with Size Tracker
328328
env:
@@ -635,6 +635,7 @@ jobs:
635635
env:
636636
MOUNT_DIR: "$HOME/blob_mnt"
637637
TEMP_DIR: "$HOME/cloudfuse_tmp"
638+
BLOCK_TEMP_DIR: "$HOME/cloudfuse_block_tmp"
638639
WORK_DIR: "."
639640
cloudfuse_ADLS_CFG: "./cloudfuse.adls.yaml"
640641
cloudfuse_ADLS_CFG_HMON: "./cloudfuse.adls.hmon.yaml"
@@ -783,30 +784,30 @@ jobs:
783784
kill $pid
784785
sleep 5
785786
786-
# - name: Create Config File - Block Blob - Block Cache
787-
# shell: bash
788-
# env:
789-
# NIGHTLY_STO_ACC_NAME: "${{ secrets.NIGHTLY_STO_BLOB_ACC_NAME }}"
790-
# NIGHTLY_STO_ACC_KEY: "${{ secrets.NIGHTLY_STO_BLOB_ACC_KEY }}"
791-
# ACCOUNT_TYPE: block
792-
# ACCOUNT_ENDPOINT: https://${{ secrets.NIGHTLY_STO_BLOB_ACC_NAME }}.blob.core.windows.net
793-
# VERBOSE_LOG: false
794-
# USE_HTTP: false
795-
# run: "./cloudfuse.test -test.v -test.coverprofile=${{ env.WORK_DIR }}/cloudfuse_gentest1.cov gen-test-config --config-file=azure_key_block_cache.yaml --container-name=${{ matrix.containerName }} --temp-path=${{ env.TEMP_DIR }} --output-file=${{ env.cloudfuse_CFG }}"
796-
797-
# - name: Block Blob Coverage - Block Cache
798-
# shell: bash
799-
# run: |-
800-
# rm -rf ${{ env.TEMP_DIR }}/*
801-
# ./cloudfuse.test -test.v -test.coverprofile=${{ env.WORK_DIR }}/cloudfuse_block_block_cache.cov mount ${{ env.MOUNT_DIR }} --config-file=${{ env.cloudfuse_CFG }} --foreground=true &
802-
# sleep 10
803-
# pid=`ps -a | grep cloudfuse | tr -s ' ' | cut -d ' ' -f2`
804-
# rm -rf ${{ env.MOUNT_DIR }}/*
805-
# cd test/e2e_tests
806-
# go test -v -timeout=7200s ./... -args -mnt-path=${{ env.MOUNT_DIR }} -tmp-path=${{ env.TEMP_DIR }} -quick-test=true -stream-direct-test=true
807-
# cd -
808-
# kill $pid
809-
# sleep 5
787+
- name: Create Config File - Block Blob - Block Cache
788+
shell: bash
789+
env:
790+
NIGHTLY_STO_ACC_NAME: "${{ secrets.NIGHTLY_STO_BLOB_ACC_NAME }}"
791+
NIGHTLY_STO_ACC_KEY: "${{ secrets.NIGHTLY_STO_BLOB_ACC_KEY }}"
792+
ACCOUNT_TYPE: block
793+
ACCOUNT_ENDPOINT: https://${{ secrets.NIGHTLY_STO_BLOB_ACC_NAME }}.blob.core.windows.net
794+
VERBOSE_LOG: false
795+
USE_HTTP: false
796+
run: "./cloudfuse.test -test.v -test.coverprofile=${{ env.WORK_DIR }}/cloudfuse_gentest1.cov gen-test-config --config-file=azure_key_block_cache.yaml --container-name=${{ matrix.containerName }} --temp-path=${{ env.BLOCK_TEMP_DIR }} --output-file=${{ env.cloudfuse_CFG }}"
797+
798+
- name: Block Blob Coverage - Block Cache
799+
shell: bash
800+
run: |-
801+
rm -rf ${{ env.BLOCK_TEMP_DIR }}/*
802+
./cloudfuse.test -test.v -test.coverprofile=${{ env.WORK_DIR }}/cloudfuse_block_block_cache.cov mount ${{ env.MOUNT_DIR }} --config-file=${{ env.cloudfuse_CFG }} --foreground=true &
803+
sleep 10
804+
pid=`ps -a | grep cloudfuse | tr -s ' ' | cut -d ' ' -f2`
805+
rm -rf ${{ env.MOUNT_DIR }}/*
806+
cd test/e2e_tests
807+
go test -v -timeout=7200s ./... -args -mnt-path=${{ env.MOUNT_DIR }} -tmp-path=${{ env.BLOCK_TEMP_DIR }} -quick-test=true -stream-direct-test=true
808+
cd -
809+
kill $pid
810+
sleep 5
810811
811812
- name: Create Config File - ADLS
812813
shell: bash
@@ -914,29 +915,29 @@ jobs:
914915
kill $pid
915916
sleep 5
916917
917-
# - name: Create Config File - S3 - Block Cache
918-
# shell: bash
919-
# env:
920-
# S3_BUCKET_NAME: "${{ secrets.S3TEST_BUCKET_NAME }}"
921-
# S3_ENDPOINT: "${{ secrets.S3TEST_ENDPOINT }}"
922-
# S3_KEY_ID: "${{ secrets.S3TEST_ACCESS_KEY }}"
923-
# S3_REGION: "${{ secrets.S3TEST_REGION }}"
924-
# S3_SECRET_KEY: "${{ secrets.S3TEST_SECRET_KEY }}"
925-
# run: "./cloudfuse.test gen-test-config --config-file=s3_key_block_cache.yaml --temp-path=${{ env.TEMP_DIR }} --output-file=${{ env.cloudfuse_CFG }}"
918+
- name: Create Config File - S3 - Block Cache
919+
shell: bash
920+
env:
921+
S3_BUCKET_NAME: "${{ secrets.S3TEST_BUCKET_NAME }}"
922+
S3_ENDPOINT: "${{ secrets.S3TEST_ENDPOINT }}"
923+
S3_KEY_ID: "${{ secrets.S3TEST_ACCESS_KEY }}"
924+
S3_REGION: "${{ secrets.S3TEST_REGION }}"
925+
S3_SECRET_KEY: "${{ secrets.S3TEST_SECRET_KEY }}"
926+
run: "./cloudfuse.test gen-test-config --config-file=s3_key_block_cache.yaml --temp-path=${{ env.BLOCK_TEMP_DIR }} --output-file=${{ env.cloudfuse_CFG }}"
926927

927-
# - name: S3 Coverage - Block Cache
928-
# shell: bash
929-
# run: |-
930-
# rm -rf ${{ env.MOUNT_DIR }}/*
931-
# rm -rf ${{ env.TEMP_DIR }}/*
932-
# ./cloudfuse.test -test.v -test.coverprofile=${{ env.WORK_DIR }}/cloudfuse_s3_block_cache.cov mount ${{ env.MOUNT_DIR }} --config-file=${{ env.cloudfuse_CFG }} --foreground=true &
933-
# sleep 10
934-
# pid=`ps -a | grep cloudfuse | tr -s ' ' | cut -d ' ' -f2`
935-
# cd test/e2e_tests
936-
# go test -v -timeout=7200s ./... -args -mnt-path=${{ env.MOUNT_DIR }} -tmp-path=${{ env.TEMP_DIR }} -quick-test=true -stream-direct-test=true
937-
# cd -
938-
# kill $pid
939-
# sleep 5
928+
- name: S3 Coverage - Block Cache
929+
shell: bash
930+
run: |-
931+
rm -rf ${{ env.MOUNT_DIR }}/*
932+
rm -rf ${{ env.BLOCK_TEMP_DIR }}/*
933+
./cloudfuse.test -test.v -test.coverprofile=${{ env.WORK_DIR }}/cloudfuse_s3_block_cache.cov mount ${{ env.MOUNT_DIR }} --config-file=${{ env.cloudfuse_CFG }} --foreground=true &
934+
sleep 10
935+
pid=`ps -a | grep cloudfuse | tr -s ' ' | cut -d ' ' -f2`
936+
cd test/e2e_tests
937+
go test -v -timeout=7200s ./... -args -mnt-path=${{ env.MOUNT_DIR }} -tmp-path=${{ env.BLOCK_TEMP_DIR }} -quick-test=true -stream-direct-test=true
938+
cd -
939+
kill $pid
940+
sleep 5
940941
941942
- name: Create Config File - S3 with Size Tracker
942943
shell: bash

‎component/block_cache/block_cache.go‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,10 @@ func (bc *BlockCache) getDefaultMemSize() uint64 {
409409
func (bc *BlockCache) CreateFile(options internal.CreateFileOptions) (*handlemap.Handle, error) {
410410
log.Trace("BlockCache::CreateFile : name=%s, mode=%d", options.Name, options.Mode)
411411

412+
flock := bc.fileLocks.Get(options.Name)
413+
flock.Lock()
414+
defer flock.Unlock()
415+
412416
f, err := bc.NextComponent().CreateFile(options)
413417
if err != nil {
414418
log.Err("BlockCache::CreateFile : Failed to create file %s", options.Name)
@@ -436,6 +440,10 @@ func (bc *BlockCache) OpenFile(options internal.OpenFileOptions) (*handlemap.Han
436440
options.Mode,
437441
)
438442

443+
flock := bc.fileLocks.Get(options.Name)
444+
flock.Lock()
445+
defer flock.Unlock()
446+
439447
attr, err := bc.NextComponent().GetAttr(internal.GetAttrOptions{Name: options.Name})
440448
if err != nil {
441449
log.Err("BlockCache::OpenFile : Failed to get attr of %s [%s]", options.Name, err.Error())
@@ -449,8 +457,7 @@ func (bc *BlockCache) OpenFile(options internal.OpenFileOptions) (*handlemap.Han
449457
log.Debug("BlockCache::OpenFile : Size of file handle.Size %v", handle.Size)
450458
bc.prepareHandleForBlockCache(handle)
451459

452-
if options.Flags&os.O_TRUNC != 0 ||
453-
(options.Flags&os.O_WRONLY != 0 && options.Flags&os.O_APPEND == 0) {
460+
if options.Flags&os.O_TRUNC != 0 {
454461
// If file is opened in truncate or wronly mode then we need to wipe out the data consider current file size as 0
455462
log.Debug("BlockCache::OpenFile : Truncate %v to 0", options.Name)
456463
handle.Size = 0
@@ -549,6 +556,10 @@ func (bc *BlockCache) FlushFile(options internal.FlushFileOptions) error {
549556
return nil
550557
}
551558

559+
flock := bc.fileLocks.Get(options.Handle.Path)
560+
flock.Lock()
561+
defer flock.Unlock()
562+
552563
options.Handle.Lock()
553564
defer options.Handle.Unlock()
554565

‎test/e2e_tests/file_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ func (suite *fileTestSuite) TestFileReadSmall() {
313313

314314
data, err := os.ReadFile(fileName)
315315
suite.NoError(err)
316-
suite.Len(suite.minBuff, len(data))
316+
suite.Len(data, len(suite.minBuff))
317317

318318
suite.fileTestCleanup([]string{fileName})
319319
}

0 commit comments

Comments
 (0)