You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "wirybeaver (via GitHub)" <gi...@apache.org> on 2023/09/01 22:33:35 UTC

[GitHub] [pinot] wirybeaver opened a new issue, #11491: Rephrase the description of PinotFS lastModified OR even deprecate it

wirybeaver opened a new issue, #11491:
URL: https://github.com/apache/pinot/issues/11491

   ````  
   /**
      * Returns the age of the file
      * @param uri location of file or directory
      * @return A long value representing the time the file was last modified, measured in milliseconds since epoch
      * (00:00:00 GMT, January 1, 1970) or 0L if the file does not exist or if an I/O error occurs
      * @throws IOException if uri is not valid or not present
      */
     long lastModified(URI uri)
         throws IOException;
   ```
   Return 0L is very error prone since user might forgot to check the result. The narrative "return 0L if the file does not exist or if an I/O error occurs" and "throws IOException if uri is not valid or not present" are irreconcilable paradoxes. 
   
   Given that some teams might already implement this interface with 0L check. Can we deprecate lastModified(URI uir) and create a new method, e.g., `modifyTime(URI uri)`


-- 
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: commits-unsubscribe@pinot.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [I] Rephrase the description of PinotFS lastModified OR even deprecate it [pinot]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed issue #11491: Rephrase the description of PinotFS lastModified OR even deprecate it
URL: https://github.com/apache/pinot/issues/11491


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] Jackie-Jiang commented on issue #11491: Rephrase the description of PinotFS lastModified OR even deprecate it

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on issue #11491:
URL: https://github.com/apache/pinot/issues/11491#issuecomment-1712034040

   How about adding an API with a second boolean flag controlling whether to throw exception or return 0 when encountering error?


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [I] Rephrase the description of PinotFS lastModified OR even deprecate it [pinot]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #11491:
URL: https://github.com/apache/pinot/issues/11491#issuecomment-6405467501

   This issue has been closed because it has been inactive for 180 days. If you believe this issue is still relevant, please feel free to reopen it or create a new issue. Thank you!


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] ankitsultana commented on issue #11491: Rephrase the description of PinotFS lastModified OR even deprecate it

Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on issue #11491:
URL: https://github.com/apache/pinot/issues/11491#issuecomment-1703396226

   @Jackie-Jiang : I saw that `PinotFS::lastModified` says that it is okay to return 0 in case of errors, but I think that could lead to major bugs in the future because usually people use lastModified to dictate decisions related to data deletion.
   
   In other words, returning 0 in case of errors would pass validations such as:
   
   ```
   System.currentTimeMillis() > pinotFS.lastModified(uri)
   ```
   
   which could lead to unintended deletions.
   
   At present this API is only called in `SegmentDeletionManager` for deletion of data under `Deleted_Segments` dir so we have been fine for now, but it could easily cause a incident from a future code-change.
   
   If changing the contract of this API is not possible, can we consider deprecating this and introducing a new API with proper semantics?


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [I] Rephrase the description of PinotFS lastModified OR even deprecate it [pinot]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #11491:
URL: https://github.com/apache/pinot/issues/11491#issuecomment-5824548075

   This issue has been automatically marked as stale because it has not had any activity in the last 150 days. It will be closed in 30 days if no further activity occurs. If this issue is still relevant, please leave a comment to keep it open. Thank you for your contributions!


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org