You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2022/03/25 18:12:16 UTC
[GitHub] [couchdb] noahshaw11 opened a new pull request #3970: Fix smoosh enqueueing not found dbs and typo
noahshaw11 opened a new pull request #3970:
URL: https://github.com/apache/couchdb/pull/3970
## Overview
Currently, smoosh can recover compaction jobs for databases that do not exist. This PR ensures that the database file exists before recovering the compaction job plus a minor typo fix.
## Testing recommendations
`make eunit apps=smoosh`
## Related Issues or Pull Requests
## Checklist
- [ ] Code is written and works correctly
- [ ] Changes are covered by tests
- [ ] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
- [ ] A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a change in pull request #3970: Fix smoosh enqueueing not found dbs and typo
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on a change in pull request #3970:
URL: https://github.com/apache/couchdb/pull/3970#discussion_r835551335
##########
File path: src/smoosh/src/smoosh_channel.erl
##########
@@ -537,3 +547,7 @@ cleanup_index_files(DbName, _Shard) ->
_ ->
ok
end.
+
+exists(Name) ->
+ FilePath = filename:join(config:get("couchdb", "database_dir", "."), Name),
+ filelib:is_regular(FilePath).
Review comment:
I was considering this function at first too. @iilyak has commented before to not call `couch_server` directly and this function continues to log the `{not_found, no_db_file}` errors which I would prefer to not happen in this case.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a change in pull request #3970: Fix smoosh enqueueing not found dbs and typo
Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #3970:
URL: https://github.com/apache/couchdb/pull/3970#discussion_r835549413
##########
File path: src/smoosh/src/smoosh_channel.erl
##########
@@ -537,3 +547,7 @@ cleanup_index_files(DbName, _Shard) ->
_ ->
ok
end.
+
+exists(Name) ->
+ FilePath = filename:join(config:get("couchdb", "database_dir", "."), Name),
+ filelib:is_regular(FilePath).
Review comment:
I think we can use this function there https://github.com/apache/couchdb/blob/5ed8d54bd834a43a41eba11524171217f141e0cb/src/couch/src/couch_server.erl#L165-L169
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a change in pull request #3970: Fix smoosh enqueueing not found dbs and typo
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on a change in pull request #3970:
URL: https://github.com/apache/couchdb/pull/3970#discussion_r835552067
##########
File path: src/smoosh/src/smoosh_channel.erl
##########
@@ -537,3 +547,7 @@ cleanup_index_files(DbName, _Shard) ->
_ ->
ok
end.
+
+exists(Name) ->
+ FilePath = filename:join(config:get("couchdb", "database_dir", "."), Name),
+ filelib:is_regular(FilePath).
Review comment:
But considering the issue with the engine here, I think using `couch_server:exists/1` is the better option.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a change in pull request #3970: Fix smoosh enqueueing not found dbs and typo
Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #3970:
URL: https://github.com/apache/couchdb/pull/3970#discussion_r835546030
##########
File path: src/smoosh/src/smoosh_channel.erl
##########
@@ -537,3 +547,7 @@ cleanup_index_files(DbName, _Shard) ->
_ ->
ok
end.
+
+exists(Name) ->
+ FilePath = filename:join(config:get("couchdb", "database_dir", "."), Name),
+ filelib:is_regular(FilePath).
Review comment:
The type of what `.couch` might. It could be a directory, for instance.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a change in pull request #3970: Fix smoosh enqueueing not found dbs and typo
Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #3970:
URL: https://github.com/apache/couchdb/pull/3970#discussion_r835546030
##########
File path: src/smoosh/src/smoosh_channel.erl
##########
@@ -537,3 +547,7 @@ cleanup_index_files(DbName, _Shard) ->
_ ->
ok
end.
+
+exists(Name) ->
+ FilePath = filename:join(config:get("couchdb", "database_dir", "."), Name),
+ filelib:is_regular(FilePath).
Review comment:
The file type might not be a file. It could be a directory, for instance.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a change in pull request #3970: Fix smoosh enqueueing not found dbs and typo
Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #3970:
URL: https://github.com/apache/couchdb/pull/3970#discussion_r835535041
##########
File path: src/smoosh/src/smoosh_channel.erl
##########
@@ -537,3 +547,7 @@ cleanup_index_files(DbName, _Shard) ->
_ ->
ok
end.
+
+exists(Name) ->
+ FilePath = filename:join(config:get("couchdb", "database_dir", "."), Name),
+ filelib:is_regular(FilePath).
Review comment:
Wouldn't this depend on the specific engine used?
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] noahshaw11 commented on a change in pull request #3970: Fix smoosh enqueueing not found dbs and typo
Posted by GitBox <gi...@apache.org>.
noahshaw11 commented on a change in pull request #3970:
URL: https://github.com/apache/couchdb/pull/3970#discussion_r835538837
##########
File path: src/smoosh/src/smoosh_channel.erl
##########
@@ -537,3 +547,7 @@ cleanup_index_files(DbName, _Shard) ->
_ ->
ok
end.
+
+exists(Name) ->
+ FilePath = filename:join(config:get("couchdb", "database_dir", "."), Name),
+ filelib:is_regular(FilePath).
Review comment:
Does the actual `.couch` file path depend on the engine?
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a change in pull request #3970: Fix smoosh enqueueing not found dbs and typo
Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #3970:
URL: https://github.com/apache/couchdb/pull/3970#discussion_r835572932
##########
File path: src/smoosh/src/smoosh_channel.erl
##########
@@ -537,3 +547,7 @@ cleanup_index_files(DbName, _Shard) ->
_ ->
ok
end.
+
+exists(Name) ->
+ FilePath = filename:join(config:get("couchdb", "database_dir", "."), Name),
+ filelib:is_regular(FilePath).
Review comment:
Yap. We don't want to break the abstraction boundary. If `couch_server:exists/1` is emitting un-useful logs we should improve it, perhaps add a `nologifmissing` option and such.
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] jaydoane merged pull request #3970: Fix smoosh enqueueing not found dbs and typo
Posted by GitBox <gi...@apache.org>.
jaydoane merged pull request #3970:
URL: https://github.com/apache/couchdb/pull/3970
--
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: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org