Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

targetcli: serialize multiple requests #140

Merged
merged 1 commit into from
Aug 1, 2019

Conversation

pkalever
Copy link
Contributor

Problem:

targetcli/rtslib cannot handle parallel requests at the moment.
Read more about this at http://bit.ly/targetcli-parallel-requests-issue

$ for i in {1..10}; do
targetcli /backstores/fileio create ${i} /tmp/file${i} 10M& done

Created fileio 1 with size 10485760
This _Backstore already exists in configFS
This _Backstore already exists in configFS
This _Backstore already exists in configFS
This _Backstore already exists in configFS
This _Backstore already exists in configFS
Created fileio 6 with size 10485760
Created fileio 2 with size 10485760
This _Backstore already exists in configFS
Created fileio 8 with size 10485760
Created fileio 9 with size 10485760

bails-out most of the time with above errors and sometimes even crashes.

Solution:

Serialize/defend the parallel requests by simply taking a wait lock at
targetcli level, so that only one request can be processed by targetcli at
any given point in time.

Signed-off-by: Prasanna Kumar Kalever prasanna.kalever@redhat.com

Problem:
-------
targetcli/rtslib cannot handle parallel requests at the moment.
Read more about this at http://bit.ly/targetcli-parallel-requests-issue

$ for i in {1..10}; do \
 targetcli /backstores/fileio create ${i} /tmp/file${i} 10M& done

Created fileio 1 with size 10485760
This _Backstore already exists in configFS
This _Backstore already exists in configFS
This _Backstore already exists in configFS
This _Backstore already exists in configFS
This _Backstore already exists in configFS
Created fileio 6 with size 10485760
Created fileio 2 with size 10485760
This _Backstore already exists in configFS
Created fileio 8 with size 10485760
Created fileio 9 with size 10485760

bails-out most of the time with above errors and sometimes even crashes.

Solution:
--------
Serialize/defend the parallel requests by simply taking a wait lock at
targetcli level, so that only one request can be processed by targetcli at
any given point in time.

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
@pkalever
Copy link
Contributor Author

@maurizio-lombardi @lxbsz please help review/test.Thanks!

@lxbsz
Copy link

lxbsz commented Jul 31, 2019

Tested it and worked for me.

# for i in {1..10}; do  targetcli /backstores/fileio create ${i} /tmp/file${i} 10M& done
[1] 19780
[2] 19781
[3] 19782
[4] 19783
[5] 19784
[6] 19785
[7] 19786
[8] 19787
[9] 19788
[10] 19789
# /tmp/file8 exists, using its size (10485760 bytes) instead
Created fileio 8 with size 10485760
/tmp/file3 exists, using its size (10485760 bytes) instead
Created fileio 3 with size 10485760
/tmp/file4 exists, using its size (10485760 bytes) instead
Created fileio 4 with size 10485760
/tmp/file6 exists, using its size (10485760 bytes) instead
Created fileio 6 with size 10485760
/tmp/file10 exists, using its size (10485760 bytes) instead
Created fileio 10 with size 10485760
/tmp/file1 exists, using its size (10485760 bytes) instead
Created fileio 1 with size 10485760
/tmp/file7 exists, using its size (10485760 bytes) instead
Created fileio 7 with size 10485760
/tmp/file2 exists, using its size (10485760 bytes) instead
Created fileio 2 with size 10485760
/tmp/file5 exists, using its size (10485760 bytes) instead
Created fileio 5 with size 10485760
/tmp/file9 exists, using its size (10485760 bytes) instead
Created fileio 9 with size 10485760

[1]   Done                    targetcli /backstores/fileio create ${i} /tmp/file${i} 10M
[2]   Done                    targetcli /backstores/fileio create ${i} /tmp/file${i} 10M
[3]   Done                    targetcli /backstores/fileio create ${i} /tmp/file${i} 10M
[4]   Done                    targetcli /backstores/fileio create ${i} /tmp/file${i} 10M
[5]   Done                    targetcli /backstores/fileio create ${i} /tmp/file${i} 10M
[6]   Done                    targetcli /backstores/fileio create ${i} /tmp/file${i} 10M
[7]   Done                    targetcli /backstores/fileio create ${i} /tmp/file${i} 10M
[8]   Done                    targetcli /backstores/fileio create ${i} /tmp/file${i} 10M
[9]-  Done                    targetcli /backstores/fileio create ${i} /tmp/file${i} 10M
[10]+  Done                    targetcli /backstores/fileio create ${i} /tmp/file${i} 10M

@pkalever
Copy link
Contributor Author

pkalever commented Aug 1, 2019

@lxbsz thank you for testing it.
JFI, I have tested this with 250 devices and it works for me well locally.

@maurizio-lombardi
Copy link
Collaborator

Looks ok to me

@maurizio-lombardi maurizio-lombardi merged commit 0b4fcdd into open-iscsi:master Aug 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants