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

Add db maintenance options, change wording and add maintenance debug messages for better maintenance UX #4379

Merged
merged 6 commits into from Mar 6, 2020

Conversation

johnny-bit
Copy link
Member

@johnny-bit johnny-bit commented Feb 27, 2020

As evidenced by #4376 and recent comments in https://discuss.pixls.us/t/darktable-periodic-database-maintenance/16375 it seems that on startup default for db maintenance makes more sense from UX standpoint.

This PR makes several changes:

  • adds 3 new options to perform maintenance without asking (received several requests for that)
  • changes default option to on startup, since clicking later is no-op always
  • better explains what'll happen when clicking later
  • assures user of possibility to change option easily

@TurboGit
Copy link
Member

I really contest that as a good default. Having a software slowing your work is not good at all. When you fire darktable it is because you need to do some work. This looks like M$ way of doing updates, requiring people to wait hours when starting the PC. Not good to me. I can wait when closing dt as at this point I can move on something else and let dt do its work.

So, to have this in we'll need more feedback.

@TurboGit TurboGit self-requested a review February 27, 2020 18:01
@TurboGit TurboGit added the controversial this raises concerns, don't move on the technical work before reaching a consensus label Feb 27, 2020
@rabauke
Copy link
Contributor

rabauke commented Feb 27, 2020

I really contest that as a good default. Having a software slowing your work is not good at all. When you fire darktable it is because you need to do some work. This looks like M$ way of doing updates, requiring people to wait hours when starting the PC. Not good to me. I can wait when closing dt as at this point I can move on something else and let dt do its work.

So, to have this in we'll need more feedback.

The same argument applies more or less to the option to do db maintenance while finishing darktable. When I am finished, I am finished and want to turn off my laptop now. Actually, both options have their inconveniences.

Unless you have a very large data base (where maintenance may take a bit longer), the bothersome part is not the actual db maintenance, it is the question about db maintenance that the user is asked and the decision that the user has to make. Is there any serious reason a user might want to push "later" here? Darktable is optimizing something here. So, go for it! Not doing the maintenance is probably not an option for the user in the longer run anyway, because users will get annoyed by being asked again and again. Thus, I wonder

  • do we really have to ask the user if db maintenance should be done?
  • can we not just do the db maintenance during start up and indicating maintenance by a message box.

@TurboGit
Copy link
Member

Unless you have a very large data base (where maintenance may take a bit longer), the bothersome part is not the actual db maintenance, it is the question about db maintenance that the user is asked and the decision that the user has to make. Is there any serious reason a user might want to push "later" here?

On my side, 55k images the maintenance takes 10s to 15s. And yes there is plenty of situation where you do not want to have this happening when starting and at least have a way to skip for later:

  • you fire dt to present your work to a client
  • you fire dt to start a course about dt
  • and possible more...

You certainly don't want to have to wait 15s at these stages.

@TurboGit
Copy link
Member

The same argument applies more or less to the option to do db maintenance while finishing darktable.

Well, when you first dt there is 100% chance all the time that you need to do some work (or maybe you clicked on the icon by mistake?).

When you leave dt there is lot of chance that you can wait a bit, and indeed leaving to take the train is one reason. But that's only one and only for people using a laptop, there is to my view lot more chance that you could wait when leaving dt.

@TurboGit
Copy link
Member

That being said, if we have [later] as option and more people think it is better at start why not! This is certainly not a very important point.

@parafin
Copy link
Member

parafin commented Feb 27, 2020

As a compromise darktable can somehow display a non-blocking suggestion to perform database optimization after starting up. It should be visible but not getting in the way of work. I'm not sure though how to fit it into darktable's UI:))

@ptilopteri
Copy link

openSUSE Tumbleweed 20200226
NVIDIA GF106 [GeForce GTS 450], 390.132
darktable 3.1.0~git759.bf2f465ac
OpenCL on
i7 12-core 36GB

ls library.db
-rwxrwxrwx 1 paka users 702611456 Feb 27 16:43 library.db

time sqlite3 ./library.db "VACUUM"
real 0m17.583s
user 0m1.025s
sys 0m2.276s

I definitely do not want any operation of this duration automagically taking control

@johnny-bit
Copy link
Member Author

The same argument applies more or less to the option to do db maintenance while finishing darktable. When I am finished, I am finished and want to turn off my laptop now. Actually, both options have their inconveniences.

This is typical problem of default setting and workflow. In my workflow and daily life I usually edit photos as a "break" from my usually hectic work, so having asking for db maintenance at start was for me not so good, because I can do it on closing and let dt do it's job in bg while I get back to my work. Then again, for people who do edits as last part of their workday (eg i edited photos and want to close dt) having asking at startup is OK because you can always click later and have it ask you simply on next startup... Actually having 4 options: never, on startup, on close, on both to choose from and by default match my (and my wife's) workflow was good startup point.

* do we really have to asked the user if db maintenance should be done?

Yes. My "normal" db is just 30mb (but used to be 100mb). I've seen others with db in the size exceeding 1GB. for them maintenance task will definitelly take "forever" and it's database blocking operation.

On my side, 55k images the maintenance takes 10s to 15s. And yes there is plenty of situation where you do not want to have this happening when starting and at least have a way to skip for later:

How I love confirmations popping up when I'm writting response 😄

That being said, if we have [later] as option and more people think it is better at start why not! This is certainly not a very important point.

I think with "later" I'd need to clarify message in popup, sorta like "do you want to proceed now? If not, click later and we'll ask again %s" where %s is either on next startup, next time you'll be quiting darktable or (when both is marked and message is on startup) upon closing darktable

Would you be comfortable with that? However I think I'd have to write out whole message so it's translatable and gramatically correct for all translations.

As a compromise darktable can somehow display a non-blocking suggestion to perform database optimization after starting up.

When I first coded db maintenance stuff I was scarred of UI. I still am... And the only place for that, that I'd see is some nag on top bar and I'm NOT coding that just yet. I initially wanted that as a button in options gui but I noped out of that 😛

I definitely do not want any operation of this duration automagically taking control

I'd never do that... Although running PRAGMA optimize is done unconditionally on every dt exit because it should nalmost never affect time and it's either no-op (so takes no time) or little op and has runtime benefit.

@TurboGit
Copy link
Member

At this stage I would recommend to let the current default and wait for more feedback. If you agree, please close this PR. Thanks.

@johnny-bit
Copy link
Member Author

I'll fix up the wording to better explain what "later" does (that's better UX to always be clear with user).

I queried some of my UX peers and I'd like to introduce more code around it (and options :/) I'll do that today if you don't mind.

@johnny-bit
Copy link
Member Author

Changed. Can you comment on language choosen?

src/common/database.c Outdated Show resolved Hide resolved
src/common/database.c Outdated Show resolved Hide resolved
@johnny-bit
Copy link
Member Author

OK, done - should be ready to merge.

@TurboGit TurboGit added feature: enhancement current features to improve scope: codebase making darktable source code easier to manage and removed controversial this raises concerns, don't move on the technical work before reaching a consensus labels Feb 29, 2020
@TurboGit TurboGit added this to the 3.2 milestone Feb 29, 2020
Copy link
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry not yet done.

You have introduced message on stderr for non error message. In dt stderr is only used for errors and there must be nothing displayed on the console otherwise.

You need to have those message displayed as part of the -d sql option only using dt_print().

src/common/database.c Outdated Show resolved Hide resolved
src/common/database.c Outdated Show resolved Hide resolved
src/common/database.c Outdated Show resolved Hide resolved
@johnny-bit
Copy link
Member Author

How about now? 😉
BTW: dt_print is great! Why I haven't noticed it before?

@johnny-bit johnny-bit changed the title Change default db maintenance option to on startup for better UX Add db maintenance options, change wording and add maintenance debug messages for better maintenance UX Mar 1, 2020
@TurboGit
Copy link
Member

TurboGit commented Mar 4, 2020

I'd like to make progress on this. If there is no string object please change the wording I have proposed (very small change but read better to me) and let this in. We can then improve the wording if possible but the new feature here should find its way to master at this point :)

@johnny-bit
Copy link
Member Author

OK, I'll finish wording and be done with it :) Pull #4421 showed me it - having those translated back at you in your own language shows all inaccuracies :)

Give me ~1h please.

@johnny-bit johnny-bit requested a review from TurboGit March 4, 2020 18:49
@TurboGit
Copy link
Member

TurboGit commented Mar 6, 2020

Works for me now! Thanks.

@TurboGit TurboGit merged commit 3c06315 into darktable-org:master Mar 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: enhancement current features to improve scope: codebase making darktable source code easier to manage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants