elangelo commented on code in PR #4182:
URL: https://github.com/apache/solr/pull/4182#discussion_r2894462148


##########
solr/solr-ref-guide/modules/deployment-guide/pages/backup-restore.adoc:
##########
@@ -672,6 +672,14 @@ The location must already exist within your S3 bucket 
before you can perform any
 Empty Location::
 If you do not want to use a sub-directory within your bucket to store your 
backup, you can use any of the following location options: `/`, `s3:/`, `s3://`.
 However the location option is mandatory and you will receive an error when 
trying to perform backup operations without it.
+
+Directory Markers::
+S3 has no native concept of directories.

Review Comment:
   Well... I got sucked into this without indeed looking at the bigger picture. 
I'm not saying it's absolutely wrong to work with PREMARKER blobs, but it would 
certainly not how I would implement it.
   The whole 'check if a file exists' is an anti-pattern anyway when working 
with 'cloud storage'. As even to check this you actually have to pay for that 
requests. It's way better to just try and deal with the error in case there is 
one. 
   I'm gonna look if it's possible to implement the way the backups themselves 
are taken and basically completely forget about the whole PREMARKER stuff. This 
should still be backwards compatibly with current backups that people rely on. 
The only restriction is that I will have to look into how the whole storage 
abstraction and see if I can make it work for both local and cloud storage 
without violating their way of working.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to