Skip to content

Adding harvester robustness #19373

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
31 changes: 31 additions & 0 deletions chia/cmds/plots.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,3 +228,34 @@ def remove_cmd(ctx: click.Context, final_dir: str) -> None:
@click.pass_context
def show_cmd(ctx: click.Context) -> None:
show_plots(ChiaCliContext.set_default(ctx).root_path)


@plots_cmd.command("refresh", help="Refresh the plot list")
@click.option("--hard", help="Clear the plot cache and do a full refresh", is_flag=True, default=False)
@click.pass_context
def refresh_cmd(ctx: click.Context, hard: bool) -> None:
"""
Refreshes the plot list, optionally clearing the cache for a full refresh
"""
root_path = ChiaCliContext.set_default(ctx).root_path
asyncio.run(refresh_plots(root_path, hard))


async def refresh_plots(root_path: Path, hard: bool = False) -> None:
"""
Refreshes the plot list, optionally clearing the cache for a full refresh
"""
from chia.cmds.cmds_util import get_any_service_client
from chia.rpc.harvester_rpc_client import HarvesterRpcClient

try:
async with get_any_service_client(HarvesterRpcClient, root_path, None) as (harvester_client, _):
if hard:
print("Performing hard refresh (clearing cache)...")
await harvester_client.hard_refresh_plots()
else:
print("Refreshing plots...")
await harvester_client.refresh_plots()
print("Plot refresh initiated. Check logs for progress.")
except Exception as e:
print(f"Failed to refresh plots: {e}")
6 changes: 6 additions & 0 deletions chia/plotting/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,3 +211,9 @@ def changed(self) -> bool:

def path(self) -> Path:
return self._path

def clear(self) -> None:
"""Clear all entries from the cache and mark it as changed."""
self._data = {}
self._changed = True
log.info(f"Cleared plot cache at {self._path}")
92 changes: 78 additions & 14 deletions chia/plotting/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from enum import Enum, IntEnum
from pathlib import Path
from typing import Any, Optional, Union
import os

from chia_rs import G1Element, PrivateKey
from chia_rs.sized_bytes import bytes32
Expand Down Expand Up @@ -111,13 +112,27 @@ def get_plot_filenames(root_path: Path) -> dict[Path, list[Path]]:
config = load_config(root_path, "config.yaml")
recursive_scan: bool = config["harvester"].get("recursive_plot_scan", DEFAULT_RECURSIVE_PLOT_SCAN)
recursive_follow_links: bool = config["harvester"].get("recursive_follow_links", False)
for directory_name in get_plot_directories(root_path, config):

# Get all plot directories from config
plot_directories = get_plot_directories(root_path, config)

# Process each directory
for directory_name in plot_directories:
try:
directory = Path(directory_name).resolve()
except (OSError, RuntimeError):
log.exception(f"Failed to resolve {directory_name}")
except (OSError, RuntimeError) as e:
log.exception(f"Failed to resolve {directory_name}: {e}")
continue

try:
# Get all plot files in this directory
plot_files = get_filenames(directory, recursive_scan, recursive_follow_links)
all_files[directory] = plot_files
except Exception as e:
Comment on lines +127 to +131
Copy link
Contributor

Choose a reason for hiding this comment

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

with your change, does get_filenames() ever throw an exception?

# If there's an error processing this directory, log it and continue with other directories
log.error(f"Error processing directory {directory}: {e}")
all_files[directory] = []
continue
all_files[directory] = get_filenames(directory, recursive_scan, recursive_follow_links)
return all_files


Expand Down Expand Up @@ -233,19 +248,68 @@ def get_filenames(directory: Path, recursive: bool, follow_links: bool) -> list[
if follow_links and recursive:
import glob

files = glob.glob(str(directory / "**" / "*.plot"), recursive=True)
for file in files:
filepath = Path(file).resolve()
if filepath.is_file() and not filepath.name.startswith("._"):
all_files.append(filepath)
try:
files = glob.glob(str(directory / "**" / "*.plot"), recursive=True)
for file in files:
try:
filepath = Path(file).resolve()
if filepath.is_file() and not filepath.name.startswith("._"):
all_files.append(filepath)
except Exception as e:
# If we can't process a specific file, log and continue
log.warning(f"Error processing file {file}: {e}")
continue
except Exception as e:
Comment on lines +251 to +262
Copy link
Contributor

Choose a reason for hiding this comment

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

it seems unnecessary to keep the glob() path. The idea is that the "manual" traversal works too, right? It's much easier to test if there's only one implementation

log.warning(f"Error during glob in directory {directory}: {e}")
# Fall back to manual recursive scanning if glob fails
try:
# Manually walk the directory tree to handle errors more gracefully
for root, _, files in os.walk(directory, followlinks=follow_links, onerror=lambda err: log.warning(f"Error walking directory \"{directory}\": {err}")):
for file in files:
if file.endswith(".plot") and not file.startswith("._"):
try:
filepath = Path(os.path.join(root, file)).resolve()
if filepath.is_file():
all_files.append(filepath)
except Exception as e:
log.exception(f"Error processing file {os.path.join(root, file)}")
continue
except Exception as e:
log.warning(f"Error during manual directory walk of {directory}: {e}")
else:
glob_function = directory.rglob if recursive else directory.glob
all_files = [
child for child in glob_function("*.plot") if child.is_file() and not child.name.startswith("._")
]
try:
if recursive:
# Use os.walk for recursive scanning to handle errors better
for root, _, files in os.walk(directory, followlinks=follow_links, onerror=lambda err: log.warning(f"Error walking directory: {err}")):
for file in files:
if file.endswith(".plot") and not file.startswith("._"):
try:
filepath = Path(os.path.join(root, file)).resolve()
if filepath.is_file():
all_files.append(filepath)
except Exception as e:
log.exception(f"Error processing file {os.path.join(root, file)}")
continue
else:
# Non-recursive case - just use glob
glob_function = directory.glob
for child in glob_function("*.plot"):
try:
if child.is_file() and not child.name.startswith("._"):
all_files.append(child)
except Exception as e:
# If we can't process a specific file, log and continue
log.exception(f"Error processing file {child}")
continue
except Exception as e:
log.exception(f"Error during directory scanning in {directory}")
# Continue rather than returning empty

log.debug(f"get_filenames: {len(all_files)} files found in {directory}, recursive: {recursive}")
except Exception as e:
log.warning(f"Error reading directory {directory} {e}")
log.exception(f"Error reading directory {directory}")
# We still return whatever files we found before the error

return all_files


Expand Down
8 changes: 8 additions & 0 deletions chia/rpc/harvester_rpc_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def get_routes(self) -> dict[str, Endpoint]:
return {
"/get_plots": self.get_plots,
"/refresh_plots": self.refresh_plots,
"/hard_refresh_plots": self.hard_refresh_plots,
Copy link
Contributor

Choose a reason for hiding this comment

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

it would seem more intuitive to add an argument to /refresh_plots, something like force=True, rather than adding a whole new endpoint. Maybe there's a good reason.

"/delete_plot": self.delete_plot,
"/add_plot_directory": self.add_plot_directory,
"/get_plot_directories": self.get_plot_directories,
Expand Down Expand Up @@ -66,6 +67,13 @@ async def refresh_plots(self, _: dict[str, Any]) -> EndpointResult:
self.service.plot_manager.trigger_refresh()
return {}

async def hard_refresh_plots(self, _: dict[str, Any]) -> EndpointResult:
# Clear the plot cache
self.service.plot_manager.cache.clear()
# Trigger a refresh
self.service.plot_manager.trigger_refresh()
return {}

async def delete_plot(self, request: dict[str, Any]) -> EndpointResult:
filename = request["filename"]
if self.service.delete_plot(filename):
Expand Down
3 changes: 3 additions & 0 deletions chia/rpc/harvester_rpc_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ async def get_plots(self) -> dict[str, Any]:
async def refresh_plots(self) -> None:
await self.fetch("refresh_plots", {})

async def hard_refresh_plots(self) -> None:
await self.fetch("hard_refresh_plots", {})

async def delete_plot(self, filename: str) -> bool:
response = await self.fetch("delete_plot", {"filename": filename})
# TODO: casting due to lack of type checked deserialization
Expand Down
Loading