search

FileSysOrg / jfileserver

Java file server with SMB, FTP/FTPS and NFS support, virtual filesystems, database filesystems
GNU Lesser General Public License v3.0
42 stars 17 forks source link

FtpServer uses a default 1s timeout #17

Closed nielsm5 closed 5 months ago

nielsm5 commented 6 months ago

Hey,

I noticed that the server runnable has a 1 second timeout. We use this library extensively to test out filesystem implementations but right now each test takes a minimum of 1 second which greatly increases our build times. https://github.com/FileSysOrg/jfileserver/blob/e1ca63bd70d74959970c4840c86de815d8e296c8/src/main/java/org/filesys/ftp/FTPServer.java#L548

Is it possible to make this value configurable?

FileSysOrg commented 6 months ago

Hi,

Looking at the code I think it could be changed to be a Thread.yield() call, as that loop just comes back round and waits for another socket connection at the accept().

jkosternl commented 6 months ago

Sounds good. Are you going to do some experiments with that suggested change, to see if that still works fine in all situations?

FileSysOrg commented 6 months ago

Yeah, I'll need to test it when I get a chance, see if I can get that done this week.

jkosternl commented 5 months ago

Did you find any time yet to verify the solution of Thread.yield() instead of a sleep? We'd appreciate it 😉

FileSysOrg commented 5 months ago

I implemented the fix and did some testing, I thought I’d checked the fix in but just checked and the fix has not been pushed to GitHub. I’m away on holiday at the moment but will be back home this weekend, I’ll get the fix pushed to GitHub, I have some other changes I’m working on for the next release but that will be a while before I’m ready to push those changes and do a new release.

jkosternl commented 5 months ago

Thanks! For us, pushing out a patch release with only this fix, would already be great 👍🏻 It is totally fine to make a patch release and later a minor/major release with bigger changes. Just my 2 cents.

FileSysOrg commented 5 months ago

I've just pushed the FTP server change, plus some changes to the FTP configuration I needed for testing with a Docker setup. The update is version 1.3.17.

jkosternl commented 5 months ago

Thanks man! The new version will be available on Maven Central? I didn't find it in Maven Central and https://www.filesys.org/maven/org/filesys/ yet.

FileSysOrg commented 5 months ago

Not on Maven Central yet, I need to do some juggling with versions, should be there later today hopefully.

FileSysOrg commented 5 months ago

JFileServer 1.3.17 is now available from Maven Central

nielsm5 commented 5 months ago

The issue can be closed, we're extremely happy with the results!

jkosternl commented 5 months ago

Thanks for fixing it @FileSysOrg. It works fine for us 👍🏻

FileSysOrg commented 5 months ago

Ok, great, I'll close this issue